feat(batch): jobStateTimeLimitActions property added #30158

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>

mazyu36 · 2024-05-15T17:13:02Z

@go-to-k
Thank you for the review. I apologize for the numerous formatting issues in the document.
I have addressed all of your comments.

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

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>

mazyu36 and others added 5 commits May 16, 2024 00:26

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

9d3210c

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

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

80efc2b

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

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

ddec3ea

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

fix: unit tests and integ tests.

5c79c65

fix: README

6496829

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
12 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(batch): jobStateTimeLimitActions property added #30158

feat(batch): jobStateTimeLimitActions property added #30158

mazyu36 commented May 12, 2024 •

edited by moelasmar

badmintoncryer May 12, 2024

mazyu36 May 12, 2024 •

edited

badmintoncryer May 12, 2024

mazyu36 May 12, 2024

badmintoncryer commented May 12, 2024

go-to-k left a comment

go-to-k May 15, 2024

mazyu36 May 15, 2024 •

edited

go-to-k May 16, 2024

go-to-k May 15, 2024

mazyu36 May 15, 2024

mazyu36 commented May 15, 2024

go-to-k left a comment

GavinZZ left a comment

mazyu36 commented May 17, 2024

mergify bot commented May 17, 2024

aws-cdk-automation commented May 17, 2024

mergify bot commented May 17, 2024

feat(batch): jobStateTimeLimitActions property added #30158

feat(batch): jobStateTimeLimitActions property added #30158

Conversation

mazyu36 commented May 12, 2024 • edited by moelasmar

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

badmintoncryer May 12, 2024

Choose a reason for hiding this comment

mazyu36 May 12, 2024 • edited

Choose a reason for hiding this comment

badmintoncryer May 12, 2024

Choose a reason for hiding this comment

mazyu36 May 12, 2024

Choose a reason for hiding this comment

badmintoncryer commented May 12, 2024

go-to-k left a comment

Choose a reason for hiding this comment

go-to-k May 15, 2024

Choose a reason for hiding this comment

mazyu36 May 15, 2024 • edited

Choose a reason for hiding this comment

go-to-k May 16, 2024

Choose a reason for hiding this comment

go-to-k May 15, 2024

Choose a reason for hiding this comment

mazyu36 May 15, 2024

Choose a reason for hiding this comment

mazyu36 commented May 15, 2024

go-to-k left a comment

Choose a reason for hiding this comment

GavinZZ left a comment

Choose a reason for hiding this comment

mazyu36 commented May 17, 2024

mergify bot commented May 17, 2024

aws-cdk-automation commented May 17, 2024

AWS CodeBuild CI Report

mergify bot commented May 17, 2024

mazyu36 commented May 12, 2024 •

edited by moelasmar

mazyu36 May 12, 2024 •

edited

mazyu36 May 15, 2024 •

edited