New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(stepfunctions-tasks): create scheduler #29458

Merged

mergify merged 77 commits into aws:main from badmintoncryer:29351-schedulerIntegration

Nov 14, 2024

Contributor

badmintoncryer commented Mar 12, 2024 •

edited

Loading

Issue # (if applicable)

Closes #29351.

Reason for this change

Although the creation of schedule task is supported, the AWS CDK currently lacks the capability to create these without a custom task definition.

Description of changes

I've introduced the EventBridgeSchedulerCreateScheduleTask class to address this gap.

The original issue discussed the need for both creating and updating schedules. However, to maintain focus and simplicity, this PR will only cover the creation aspect. A subsequent PR will be dedicated to schedule updates.

Description of how you validated changes

I have added both integ and unit tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

github-actions bot added effort/medium feature-request p2 labels

aws-cdk-automation requested a review from a team

March 12, 2024 06:06

github-actions bot added the star-contributor label

Contributor Author

badmintoncryer commented Mar 12, 2024

@cshields236 Please wait for a while.

aws-cdk-automation previously requested changes

View reviewed changes

Collaborator

aws-cdk-automation left a comment

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

badmintoncryer had a problem deploying to test-pipeline

March 13, 2024 14:46

— with

GitHub Actions Failure

badmintoncryer had a problem deploying to test-pipeline

March 14, 2024 00:25

— with

GitHub Actions Failure

badmintoncryer had a problem deploying to test-pipeline

March 14, 2024 01:27

— with

GitHub Actions Failure

aws-cdk-automation dismissed their stale review

March 14, 2024 01:28

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

badmintoncryer had a problem deploying to test-pipeline

March 14, 2024 01:43

— with

GitHub Actions Failure

badmintoncryer marked this pull request as ready for review

March 14, 2024 01:43

badmintoncryer force-pushed the 29351-schedulerIntegration branch 2 times, most recently from 562fae8 to 6a21c6d Compare

March 15, 2024 04:36

aws-cdk-automation added the pr/needs-community-review label

github-actions bot mentioned this pull request

Weekly PR metrics report #29527

Closed

github-actions bot mentioned this pull request

Monthly PR metrics report #29668

Closed

lpizzinidev suggested changes

View reviewed changes

Contributor

lpizzinidev left a comment

Thanks 👍 Left comments for adjustments

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated

+                 *
+                 * @see https://docs.aws.amazon.com/scheduler/latest/UserGuide/schedule-types.html
+                 */
+                readonly scheduleExpression: string;

Contributor

lpizzinidev Jun 12, 2024

What about using a Schedule class similar to this?

Contributor Author

badmintoncryer Jun 22, 2024

It's a really nice suggestion. Do you mean implementing the Schedule class in create-schedule.ts? Or importing this class directly?

Contributor

lpizzinidev Jun 26, 2024

I see that the class is declared in multiple packages already.
I think you can create it inside the aws-stepfunctions-tasks/lib package.

Contributor Author

badmintoncryer Jul 4, 2024 •

edited

Loading

I defined Schedule class in create-schedule.ts.

I also referred to this class, particularly the rate() method.
https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-events/lib/schedule.ts#L11

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md Outdated Show resolved Hide resolved

aws-cdk-automation removed the pr/needs-community-review label

badmintoncryer force-pushed the 29351-schedulerIntegration branch from 21bf7d1 to 1c9a271 Compare

June 17, 2024 13:47

badmintoncryer commented

View reviewed changes

Contributor Author

badmintoncryer left a comment •

edited

Loading

Thank you for your review!! I've addressed your comments.
I have one question about implementation of Schedule class.

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated

+                 *
+                 * @see https://docs.aws.amazon.com/scheduler/latest/UserGuide/managing-targets.html
+                 */
+                readonly target: string;

Contributor Author

badmintoncryer Mar 14, 2024

I want to name this parameter as targetArn but it causes linter failure.

error: [awslint:props-no-arn-refs:aws-cdk-lib.aws_stepfunctions_tasks.EventBridgeSchedulerCreateScheduleTaskProps.targetArn] props must use strong types instead of attributes. props should not have "arn" suffix

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated

+                 *
+                 * @see https://docs.aws.amazon.com/scheduler/latest/UserGuide/schedule-types.html
+                 */
+                readonly scheduleExpression: string;

Contributor Author

badmintoncryer Jun 22, 2024

It's a really nice suggestion. Do you mean implementing the Schedule class in create-schedule.ts? Or importing this class directly?

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts Outdated Show resolved Hide resolved

badmintoncryer added 8 commits

June 22, 2024 16:59


          feat: add create-schedule.ts

353c51f


          feat: updte create-schedule.ts

1451fc7


          feat: add validation

d6ba324


          feat: export

fcce464


          test: integ test

7681b18


          test: update integ test

728ed56


          test: add unit test


          test: add unit test

d2940e8

Contributor

mergify bot commented Nov 12, 2024

update

☑️ Nothing to do

#commits-behind > 0 [📌 update requirement]
-closed [📌 update requirement]
-conflict [📌 update requirement]
queue-position = -1 [📌 update requirement]

codecov bot commented Nov 12, 2024 •

edited

Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.17%. Comparing base (92bfb59) to head (2ccf2e8).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #29458   +/-   ##
=======================================
  Coverage   77.17%   77.17%           
=======================================
  Files         105      105           
  Lines        7162     7162           
  Branches     1311     1311           
=======================================
  Hits         5527     5527           
  Misses       1455     1455           
  Partials      180      180

Flag	Coverage Δ
suite.unit	`77.17% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`77.17% <ø> (ø)`

Contributor

paulhcsun commented Nov 12, 2024 •

edited

Loading

Hi @badmintoncryer, we've recently added codecov for checking coverage for CLI related files and it seems to be blocking this PR even though it is an unrelated file failing: packages/aws-cdk/lib/api/logs/logs-monitor.ts. I will check with the team to see if we can manually merge or if we need to take any additional action. Either way I don't expect that you will need to fix that coverage for this PR. Thanks for your patience while we figure this out.

aws-cdk-automation removed the pr/needs-maintainer-review label

Contributor Author

badmintoncryer commented Nov 13, 2024

@paulhcsun Thank you very much for your assistance!

iliapolo mentioned this pull request

chore: exclude some logs-monitor code from coverage #32107

Merged


          Merge branch 'main' into 29351-schedulerIntegration

9b92425

mergify bot dismissed paulhcsun’s stale review

November 13, 2024 09:41

Pull request has been modified.

aws-cdk-automation added the pr/needs-maintainer-review label

Contributor

paulhcsun commented Nov 13, 2024

@Mergifyio update


          Merge branch 'main' into 29351-schedulerIntegration

8fe78dd

Contributor

mergify bot commented Nov 13, 2024

update

✅ Branch has been successfully updated

paulhcsun previously approved these changes

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review label


          Merge branch 'main' into 29351-schedulerIntegration

0753f0b

mergify bot dismissed paulhcsun’s stale review

November 14, 2024 05:56

Pull request has been modified.

aws-cdk-automation added the pr/needs-maintainer-review label

Contributor

paulhcsun commented Nov 14, 2024

@Mergifyio update

Contributor

mergify bot commented Nov 14, 2024

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/codecov.yml without workflows permission


          Merge branch 'main' into 29351-schedulerIntegration

2ccf2e8

paulhcsun approved these changes

View reviewed changes

Contributor

paulhcsun commented Nov 14, 2024

@Mergifyio update

Contributor

mergify bot commented Nov 14, 2024

update

☑️ Nothing to do

#commits-behind > 0 [📌 update requirement]
-closed [📌 update requirement]
-conflict [📌 update requirement]
queue-position = -1 [📌 update requirement]

Contributor

paulhcsun commented Nov 14, 2024

Ok it should hopefully work this time 🤞

Collaborator

aws-cdk-automation commented Nov 14, 2024

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 2ccf2e8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Contributor

mergify bot commented Nov 14, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify bot merged commit d14a01c into aws:main

18 checks passed

github-actions bot commented Nov 14, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot locked as resolved and limited conversation to collaborators

badmintoncryer deleted the 29351-schedulerIntegration branch

November 14, 2024 21:36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

effort/medium feature-request p2 pr/needs-maintainer-review star-contributor