-
Notifications
You must be signed in to change notification settings - Fork 29
Migrate to AWS SDK V3 - Part 1 #65
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
lib/configuration/variables/sources/instance-dependent/get-aws.js
Outdated
Show resolved
Hide resolved
dc02d82
to
79e9aa0
Compare
return { value: Account }; | ||
const provider = serverlessInstance.getProvider('aws'); | ||
const result = await provider.request('STS', 'getCallerIdentity', {}, { useCache: true }); | ||
return { value: result.Account }; |
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.
Can we revert formatting changes like these? This will make the diff much smaller, and also will minimize changes with upstream. It seems like there are 4-5 files impacted.
ListAttachedRolePoliciesCommand, | ||
CreateRoleCommand, | ||
AttachRolePolicyCommand, | ||
} = require('@aws-sdk/client-iam'); | ||
const AWSClientFactory = require('../../../../../aws/client-factory'); |
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.
Aren't these files the custom resource handlers?
If yes, they shouldn't be impacted by the changes in the CLI. And they wouldn't be able to require files from the CLI package (because they would run in Lambda).
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.
Ah, this was a fix to make sure they are re-using the same client.
This PR adds AWS SDK v3 support for everything. Opt in to v3 with
SLS_AWS_SDK_V3=1
. I am running the CI on both versions in the Node 22 job... however mostly things are mocked, so this is not testing much. A notable difference is that the v2 SDK has memoization that we expose withuseCache
. We are not doing that for v3. Do we want that?In a follow-up PR we could flip the default (Part 2), and in another follow-up, remove v2 support (Part 3).
Smashing through this with Claude Code. 🤖