-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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.
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
feat(ses): maximum delivery time for emails #32102
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
* | ||
* This value must be greater than or equal to 5 minutes and less than or equal to 14 hours. | ||
* | ||
* @default - 14 hours |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This default value is described at Advanced delivery options/Maximum delivery duration section.
Setting a custom maximum delivery limit (shorter than the SES default of 14 hours),
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor feedbacks.
* | ||
* This value must be greater than or equal to 5 minutes and less than or equal to 14 hours. | ||
* | ||
* @default - 14 hours |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd be specific here, @default undefined - SES defaults to 14 hours
if (props.maxDeliveryDuration.toMilliseconds() < Duration.seconds(1).toMilliseconds()) { | ||
throw new Error(`The maximum delivery duration must be greater than or equal to 5 minutes (300 seconds), got: ${props.maxDeliveryDuration.toMilliseconds()} milliseconds.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check needed if we already have the 5 min check?
const configurationSet = new CfnConfigurationSet(this, 'Resource', { | ||
deliveryOptions: undefinedIfNoKeys({ | ||
sendingPoolName: props.dedicatedIpPool?.dedicatedIpPoolName, | ||
tlsPolicy: props.tlsPolicy, | ||
maxDeliverySeconds: props.maxDeliveryDuration?.toSeconds(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happen if it's an unresolved token and calling toSeconds()
?
Issue # (if applicable)
None
Reason for this change
Amazon Simple Email Service (SES) offers a new delivery option that allows us to set a custom maximum delivery time for our emails at Oct 15, 2024
Cfn documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ses-configurationset-deliveryoptions.html#cfn-ses-configurationset-deliveryoptions-maxdeliveryseconds
Description of changes
Add
maxDeliveryDuration
toConfigurationSetProps
.Description of how you validated changes
Add both unit and integ tests.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license