feat(scheduler): start and end time for schedule construct #28306

sakurai-ryo · 2023-12-08T17:43:13Z

This PR added support for start and end time of the schedule.

Description

Currently, users cannot set a start time and an end time for the schedule.
A schedule without a start date will begin as soon as it is created and available, and without an end date, it will continue to invoke its target indefinitely.
With this feature, users can set the start and end dates of a schedule, allowing for more flexible schedule configurations.
https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-scheduler-schedule.html#cfn-scheduler-schedule-startdate
https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-scheduler-schedule.html#cfn-scheduler-schedule-enddate

In CloudFormation, users can use this feature as follows:

  TestSchedule:
    Type: AWS::Scheduler::Schedule
    Properties: 
      StartDate: "2024-12-01T13:09:00.000Z"
      EndDate: "2025-12-01T00:00:00.001Z"
      ScheduleExpression: "at(2023-01-01T00:00:00)"
      State: "ENABLED"
      Target: # target

Major changes

add property to ScheduleProps interface

Added startDate and endDate properties, and typed these values as string based on the following PR comments.
#26819 (comment)

It is not necessary to specify both startDate and endDate, they can be set independently.
Validation is performed on the following points.

Error if not following ISO 8601 format.
- Must include milliseconds (yyyy-MM-ddTHH:mm:ss.SSSZ)
Error if startDate is later than endDate

If a time before the current time is specified, the following error occurs in CFn, but no validation is performed because the timing of validation in CDK and the timing of actual deployment are different.
The StartDate you specify cannot be earlier than 5 minutes ago.

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

…scheduler-time-frame

lpizzinidev

Looks good overall 👍
I left some suggestions for minor improvements.

lpizzinidev · 2023-12-09T15:36:43Z

packages/@aws-cdk/aws-scheduler-alpha/lib/schedule.ts

@@ -213,6 +233,8 @@ export class Schedule extends Resource implements ISchedule {
    return this.metricAll('InvocationsSentToDeadLetterCount_Truncated_MessageSizeExceeded', props);
  }

+  private static readonly ISO8601_REGEX = /^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[01]|0[1-9]|[12][0-9])T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])\.[0-9]{3}(Z)?$/;


Suggested change

private static readonly ISO8601_REGEX = /^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[01]|0[1-9]|[12][0-9])T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])\.[0-9]{3}(Z)?$/;

private static readonly ISO8601_REGEX = /^([0-2]\d{3})-(0[0-9]|1[0-2])-([0-2]\d|3[01])T([01]\d|2[0-4]):([0-5]\d):([0-6]\d)((\.\d{3})?)Z$/;

A previous issue was caused by TransitionDate being formatted without the final Z so a different regex may be better in this case.

You are absolutely right.
I made some modifications from your suggestion because CFn throws the following error if it does not include milliseconds.
StartDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ

lpizzinidev · 2023-12-09T15:43:59Z

packages/@aws-cdk/aws-scheduler-alpha/lib/schedule.ts

+  private validateTimeFrame(startDate?: string, endDate?: string) {
+    if (startDate && !Schedule.ISO8601_REGEX.test(startDate)) {
+      throw new Error(`startDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${startDate}`);


Suggested change

throw new Error(`startDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${startDate}`);

throw new Error(`startDate must be in ISO 8601 format, got ${startDate}`);

lpizzinidev · 2023-12-09T15:44:21Z

packages/@aws-cdk/aws-scheduler-alpha/lib/schedule.ts

+    }
+    if (endDate && !Schedule.ISO8601_REGEX.test(endDate)) {
+      throw new Error(`endDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${endDate}`);


Suggested change

throw new Error(`endDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${endDate}`);

throw new Error(`endDate must be in ISO 8601 format, got ${endDate}`);

lpizzinidev · 2023-12-09T15:46:27Z

packages/@aws-cdk/aws-scheduler-alpha/lib/schedule.ts

+      const start = new Date(startDate);
+      const end = new Date(endDate);
+
+      if (end <= start) {
+        throw new Error(`startDate must come before the endDate but got startDate: ${startDate}, endDate: ${endDate}`);
+      }
+    }


Suggested change

if (startDate && endDate) {

const start = new Date(startDate);

const end = new Date(endDate);

if (end <= start) {

throw new Error(`startDate must come before the endDate but got startDate: ${startDate}, endDate: ${endDate}`);

}

}

if (startDate && endDate && startDate > endDate) {

throw new Error(`startDate must preceed endDate, got startDate: ${startDate}, endDate: ${endDate}`);

}

It should be fine to compare the strings directly given the format.

lpizzinidev · 2023-12-09T15:50:59Z

packages/@aws-cdk/aws-scheduler-alpha/README.md

+### Configuring a start and end time of the Schedule
+
+If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the startDate and endDate.
+These values must follow the format `yyyy-MM-ddTHH:mm:ss.SSSZ`.


Suggested change

These values must follow the format `yyyy-MM-ddTHH:mm:ss.SSSZ`.

These values must follow the ISO 8601 format (`yyyy-MM-ddTHH:mm:ss.SSSZ`).

sakurai-ryo · 2023-12-11T12:15:45Z

@lpizzinidev
Thanks for your review! I fixed them.

lpizzinidev

Nice 👍

kaizencc · 2023-12-11T19:42:20Z

packages/@aws-cdk/aws-scheduler-alpha/README.md

@@ -138,6 +138,21 @@ new Schedule(this, 'Schedule', {
 });
 ```

+### Configuring a start and end time of the Schedule
+
+If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the startDate and endDate.


Suggested change

If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the startDate and endDate.

If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the `startDate` and `endDate`.

I would also prefer the props to be just named start and end, or startTime/endTime.

In CFn, it is StartDate and EndDate, so I shortened it to start and end.

kaizencc · 2023-12-11T19:47:37Z

packages/@aws-cdk/aws-scheduler-alpha/README.md

+    startDate: '2023-01-01T00:00:00.000Z',
+    endDate: '2023-02-01T00:00:00.000Z',


I think we standardized on using the Date type a while back. Similar to what we are doing here. We can then convert to whatever format scheduler expects.

How do you feel about that? I think that's the best course of action here as it allows for more types of inputs, but feel free to push back.

Thanks for the comment!

Using the Date type to allow for a variety of user inputs is certainly appealing.
However, I have some reservations about it, primarily due to potential issues with timezones.
When we initialize a Date object with a string that lacks timezone specification(new Date("2022-08-04T15:19:46.125")), the system's timezone is used by default.
For instance, if a local machine is set to Asia/Tokyo and the CI environment to UTC, the same time would be offset by 9 hours between these environments.
Since we are only dealing with ISO 8601 format and exclusively in UTC for this properties, using a string might be a safer option to avoid these timezone pitfalls.

By typing these values as string representation in the ISO 8601 format, we could mitigate these inconsistencies and ensure more predictable behavior across different environments.

This is just my personal opinion and I would love to get feedback from the CDK team.

CFN accepts startDate only in UTC time. I think when we do new Date().toISOString() the Date is automatically converted to UTC time, specifically ISO 8601, which is what we want.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString

Yes, toISOString method is used to convert to ISO 8601 format (with UTC timezone), but in the previous comment I was referring to the timezone at the initialization of the Date object.

However, I would like to change these properties to use Date type, since this problem does not occur if the user passes a string with timezone at initialization, and this is not a CDK problem but a JavaScript Date class specification problem.
Sorry for the confusion.

Pull request has been modified.

sakurai-ryo · 2023-12-12T04:28:31Z

@kaizencc
Thanks for your review!
I fixed it and left some comments.

…scheduler-time-frame

sakurai-ryo · 2023-12-13T12:40:15Z

@kaizencc
Thanks! Changed the start and end property types to use Date type.
#28306 (comment)

kaizencc

Lovely @sakurai-ryo, thanks for your contribution!

mergify · 2023-12-13T18:13:13Z

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 · 2023-12-13T18:40:50Z

AWS CodeBuild CI Report

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

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

mergify · 2023-12-13T18:41:40Z

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

sakurai-ryo added 9 commits December 1, 2023 00:09

feat(scheduler): add support for scheduler time frame

8acc52a

feat(scheduler): readme

883ba81

feat(scheduler): fix test

a8c0232

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

ac47212

…scheduler-time-frame

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

cece795

…scheduler-time-frame

feat(scheduler): fix readme

029d40b

feat(scheduler): fix comments

ac5c8d2

feat(scheduler): minor changes in tests

16d0f02

feat(scheduler): add new line

8cc2d91

aws-cdk-automation requested a review from a team December 8, 2023 17:43

github-actions bot added p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK labels Dec 8, 2023

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

e6c41fa

…scheduler-time-frame

sakurai-ryo changed the title ~~feat(scheduler): add support for start and end time of the schedule~~ feat(scheduler): add support for start and end time of the schedule construct Dec 9, 2023

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

lpizzinidev suggested changes Dec 9, 2023

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 Dec 9, 2023

sakurai-ryo added 2 commits December 11, 2023 20:14

feat(scheduler): fix for the review

e17d2b6

feat(scheduler): fix build

e037852

lpizzinidev approved these changes Dec 11, 2023

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 11, 2023

kaizencc changed the title ~~feat(scheduler): add support for start and end time of the schedule construct~~ feat(scheduler): start and end time for schedule construct Dec 11, 2023

kaizencc previously requested changes Dec 11, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 11, 2023

feat(scheduler): rename properties

53bba92

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 12, 2023

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

380f428

…scheduler-time-frame

feat(scheduler): typing start and end as Date

a56f918

kaizencc approved these changes Dec 13, 2023

View reviewed changes

Merge branch 'main' into scheduler-time-frame

2f12a18

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 13, 2023

mergify bot merged commit 0b4ab1d into aws:main Dec 13, 2023
12 checks passed

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

Monthly PR metrics report - Dec 2023 #28544

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(scheduler): start and end time for schedule construct #28306

feat(scheduler): start and end time for schedule construct #28306

sakurai-ryo commented Dec 8, 2023

lpizzinidev left a comment

lpizzinidev Dec 9, 2023

sakurai-ryo Dec 11, 2023

lpizzinidev Dec 9, 2023

lpizzinidev Dec 9, 2023

lpizzinidev Dec 9, 2023

lpizzinidev Dec 9, 2023

sakurai-ryo commented Dec 11, 2023

lpizzinidev left a comment

kaizencc Dec 11, 2023

kaizencc Dec 11, 2023

sakurai-ryo Dec 12, 2023

kaizencc Dec 11, 2023

sakurai-ryo Dec 12, 2023

kaizencc Dec 12, 2023

sakurai-ryo Dec 13, 2023

sakurai-ryo commented Dec 12, 2023

sakurai-ryo commented Dec 13, 2023 •

edited

Loading

kaizencc left a comment

mergify bot commented Dec 13, 2023

aws-cdk-automation commented Dec 13, 2023

mergify bot commented Dec 13, 2023

	private static readonly ISO8601_REGEX = /^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]\|0[1-9])-(3[01]\|0[1-9]\|[12][0-9])T(2[0-3]\|[01][0-9]):([0-5][0-9]):([0-5][0-9])\.[0-9]{3}(Z)?$/;
	private static readonly ISO8601_REGEX = /^([0-2]\d{3})-(0[0-9]\|1[0-2])-([0-2]\d\|3[01])T([01]\d\|2[0-4]):([0-5]\d):([0-6]\d)((\.\d{3})?)Z$/;

	throw new Error(`startDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${startDate}`);
	throw new Error(`startDate must be in ISO 8601 format, got ${startDate}`);

	throw new Error(`endDate needs to follow the format yyyy-MM-ddTHH:mm:ss.SSSZ but got ${endDate}`);
	throw new Error(`endDate must be in ISO 8601 format, got ${endDate}`);

	These values must follow the format `yyyy-MM-ddTHH:mm:ss.SSSZ`.
	These values must follow the ISO 8601 format (`yyyy-MM-ddTHH:mm:ss.SSSZ`).

	If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the startDate and endDate.
	If you choose a recurring schedule, you can set the start and end time of the Schedule by specifying the `startDate` and `endDate`.

		startDate: '2023-01-01T00:00:00.000Z',
		endDate: '2023-02-01T00:00:00.000Z',

feat(scheduler): start and end time for schedule construct #28306

feat(scheduler): start and end time for schedule construct #28306

Conversation

sakurai-ryo commented Dec 8, 2023

Description

Major changes

add property to ScheduleProps interface

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Dec 11, 2023

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Dec 12, 2023

sakurai-ryo commented Dec 13, 2023 • edited Loading

kaizencc left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 13, 2023

aws-cdk-automation commented Dec 13, 2023

AWS CodeBuild CI Report

mergify bot commented Dec 13, 2023

sakurai-ryo commented Dec 13, 2023 •

edited

Loading