Migrate to AWS SDK V3 - Part 1#65
Conversation
GrahamCampbell
force-pushed
the
aws-sdk-v3
branch
from
dc02d82 to
79e9aa0
Compare
| CreateRoleCommand, | ||
| AttachRolePolicyCommand, | ||
| } = require('@aws-sdk/client-iam'); | ||
| const AWSClientFactory = require('../../../../../aws/client-factory'); |
There was a problem hiding this comment.
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.
Ah, this was a fix to make sure they are re-using the same client.
mnapoli
left a comment
mnapoli
left a comment
There was a problem hiding this comment.
I went through the changes and did some basic tests, things seem to work with the v3 SDK. I also cleaned up the diff.
The last thing we need to figure out for me are the custom resources: I don't see why they are impacted here. I'd revert them entirely but I'm not sure if I'm missing something, so I'll wait for your take on this.
|
The changes to those are not strictly necessary, but they should improve performance by re-using the TPC connections. |
|
@GrahamCampbell but wait I don't understand at all. Custom resource handlers are deployed in AWS lambda in users accounts. They don't have the whole codebase of serverless framework, nor its dependencies. I don't see how it's possible that a custom resource handler requires from the rest of the codebase, that makes no sense to me. https://github.com/oss-serverless/serverless/blob/1f3039673266a4bd88188ef4c8aa728c8cfeb6c8/lib/plugins/aws/custom-resources/resources/README.md#L4-L3 |
|
OK, sure. We can revert those changes. |
2 participants
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. 🤖