feat(batch): jobStateTimeLimitActions property added by mazyu36 · Pull Request #30158 · aws/aws-cdk

mazyu36 · 2024-05-12T01:18:25Z

Issue # (if applicable)

Closes #30142 .

Reason for this change

Missing property in the L2 Construct.

Description of changes

Add jobStateTimeLimitActions property to the JobQueue Construct.

Description of how you validated changes

Add unit tests and integ 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

badmintoncryer · 2024-05-12T04:25:10Z

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

+ * The action to take when a job is at the head of the job queue in the specified state
+ * for the specified period of time.
+ */
+  readonly action: JobStateTimeLimitActionsAction;


It may be better to make action optional and set the default value to JobStateTimeLimitActionsAction.CANCEL, since that is the only value we can configure at the moment.

Thank you for your comment.
I think your suggestion is better.

I have changed action to be optional and set its default value to JobStateTimeLimitActionsAction.CANCEL.

badmintoncryer · 2024-05-12T04:26:34Z

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

+  /**
+   * The state of the job needed to trigger the action.
+   */
+  readonly state: JobStateTimeLimitActionsState;


This is similar comment above, it may be better to make state optional and set default value to JobStateTimeLimitActionsState.RUNNABLE.

Thanks.

I have changed state to be optional and set its default value to JobStateTimeLimitActionsState.RUNNABLE.

badmintoncryer · 2024-05-12T04:28:23Z

I am not a reviewer, but I have provided some comments for your reference.

go-to-k

Thank you for this PR.
Sorry for the many comments, but I commented as much as I could see.
I may make additional comments if anything else comes up.

packages/aws-cdk-lib/aws-batch/README.md

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

go-to-k · 2024-05-15T14:34:00Z

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

+ * The reason to log for the action being taken.
+ * @see https://docs.aws.amazon.com/batch/latest/userguide/troubleshooting.html#job_stuck_in_runnable
+ */
+export enum JobStateTimeLimitActionsReason {


Shouldn't we add the MISCONFIGURATION:SERVICE_ROLE_PERMISSIONS?

It seems that the MISCONFIGURATION:SERVICE_ROLE_PERMISSIONS is not supported at the moment.

I tried to set that via CDK and CLI, but I can't.

I checked in the management console as well, but it seems that only three other options are available for configuration.

I see, nice catch.

go-to-k · 2024-05-15T14:37:06Z

packages/aws-cdk-lib/aws-batch/test/job-queue.test.ts

Could you also add a new test for the case where the array is empty?

I added a test case for an empty array.

packages/aws-cdk-lib/aws-batch/README.md

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

go-to-k

Thanks for the changes, LGTM.

GavinZZ

Minor comments

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

mazyu36 · 2024-05-17T00:15:15Z

@GavinZZ
Thank you for the review !
I have responded to all of your questions.

mergify · 2024-05-17T17:33:54Z

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).

aws-cdk-automation · 2024-05-17T18:01:58Z

AWS CodeBuild CI Report

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

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

mergify · 2024-05-17T18:02:20Z

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).

aws-cdk-automation · 2024-07-25T21:09:20Z

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.

feat: jobStateTimeLimitActions property added

b2ceb41

aws-cdk-automation requested a review from a team May 12, 2024 01:18

github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels May 12, 2024

mazyu36 added 4 commits May 12, 2024 10:23

Merge branch 'main' into batch-jobstatelimitactions-30142

fb61330

fix: add docs to enum

ca0cc7f

fix: README

8524e16

fix: README

4533771

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 12, 2024

fix: typo

402d5b9

badmintoncryer reviewed May 12, 2024

View reviewed changes

refactor: set default value enum

6d610e6

github-actions bot mentioned this pull request May 13, 2024

Weekly PR metrics report #30176

Closed

go-to-k suggested changes May 15, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 15, 2024

mazyu36 and others added 11 commits May 16, 2024 00:22

Update packages/aws-cdk-lib/aws-batch/README.md

8dfc79c

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

cf68337

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

f831e9c

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

1489031

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

4701737

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

f42dbf5

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

9537e07

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

98b110f

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

8a1241e

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

2e0836e

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-batch/lib/job-queue.ts

3a629eb

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

go-to-k approved these changes May 16, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 16, 2024

GavinZZ reviewed May 16, 2024

View reviewed changes

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts Show resolved Hide resolved

packages/aws-cdk-lib/aws-batch/lib/job-queue.ts Show resolved Hide resolved

github-actions bot added bug This issue is a bug. feature-request A feature should be added or improved. and removed feature-request A feature should be added or improved. bug This issue is a bug. labels May 17, 2024

GavinZZ approved these changes May 17, 2024

View reviewed changes

Merge branch 'main' into batch-jobstatelimitactions-30142

9a4b045

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 17, 2024

mergify bot merged commit 411a58c into aws:main May 17, 2024

mazyu36 deleted the batch-jobstatelimitactions-30142 branch May 18, 2024 04:04

github-actions bot mentioned this pull request Jun 1, 2024

Monthly PR metrics report #30414

Closed

NOUIY mentioned this pull request Jun 14, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#103

Open

snyk-io bot mentioned this pull request Jun 20, 2024

[Snyk] Upgrade aws-cdk-lib from 2.138.0 to 2.143.1 Opetushallitus/heratepalvelu#280

Closed

NOUIY mentioned this pull request Jun 20, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#104

Open

snyk-io bot mentioned this pull request Jun 21, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.143.1 Opetushallitus/ovara#36

Open

This was referenced Jun 22, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#105

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#106

Open

This was referenced Jul 5, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#107

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#108

Open

This was referenced Jul 16, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#109

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#110

Open

NOUIY mentioned this pull request Jul 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#111

Open

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Conversation

mazyu36 commented May 12, 2024 • edited by moelasmar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

badmintoncryer May 12, 2024

Choose a reason for hiding this comment

Uh oh!

mazyu36 May 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

badmintoncryer May 12, 2024

Choose a reason for hiding this comment

Uh oh!

mazyu36 May 12, 2024

Choose a reason for hiding this comment

Uh oh!

badmintoncryer commented May 12, 2024

Uh oh!

go-to-k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

go-to-k May 15, 2024

Choose a reason for hiding this comment

Uh oh!

mazyu36 May 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

go-to-k May 16, 2024

Choose a reason for hiding this comment

Uh oh!

go-to-k May 15, 2024

Choose a reason for hiding this comment

Uh oh!

mazyu36 May 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

go-to-k left a comment

Choose a reason for hiding this comment

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mazyu36 commented May 17, 2024

Uh oh!

mergify bot commented May 17, 2024

Uh oh!

aws-cdk-automation commented May 17, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented May 17, 2024

Uh oh!

aws-cdk-automation commented Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mazyu36 commented May 12, 2024 •

edited by moelasmar

Loading

mazyu36 May 12, 2024 •

edited

Loading

mazyu36 May 15, 2024 •

edited

Loading