-
Notifications
You must be signed in to change notification settings - Fork 2
Switch letter_status_update lambda to use eventsub SNS topic #360
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
base: main
Are you sure you want to change the base?
Conversation
b2a5207 to
94171d9
Compare
94171d9 to
966f6eb
Compare
| return { | ||
| s3Client: new S3Client(), | ||
| sqsClient: new SQSClient(), | ||
| snsClient: new SNSClient(), |
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.
for these clients we don't specify the region.
Are we sure they are assigned the correct region?
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.
As I understand it, when you create an AWS client inside a lambda, it inherits the region from where the lambda is executed. (Which will be eu-west-2 as that's what's configured in the terraform in the nhs-notify-internal project). I suppose that might be an issue for services that are only available in a limited number of regions, but that doesn't seem to be a problem here.
e5b9274
5bf0071 to
d11f96f
Compare
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.