Skip to content
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

Migrate sdk v2 to sdk v3 #225

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

mettke
Copy link
Contributor

@mettke mettke commented Jan 5, 2024

This is a follow up PR of #222 and migrates aws sdk v2 to aws sdk v3. It also contains the commit from #222 and should only be merged afterwards.

@alexcasalboni alexcasalboni force-pushed the migrate-sdk-v2-to-sdk-v3 branch from 3fc3986 to 91360d8 Compare January 8, 2024 10:41
@alexcasalboni
Copy link
Owner

alexcasalboni commented Jan 8, 2024

I have rebased and pushed a couple of minor test improvements (to keep 100% coverage), then figured out that tests included some pretty big assumptions on how the SDK handles errors. Now I'm using the right error class instead of checking the error type/message.

Thank you again for this amazing contribution 🙏 🚀

@mettke
Copy link
Contributor Author

mettke commented Jan 8, 2024

Yeah. In fact it is quite a shame that it is not that easy to actually mock aws calls. I mean sure you have that mocking lib, but it does not give you an idea of how errors actually look like and what to expect. One is kinda left in the dark.
But your changes look good. Happy to help.

@alexcasalboni
Copy link
Owner

Indeed, that's why I started working on integration tests.

Even if the mock library isn't perfect, it does make sense to mock API calls for unit tests (and keep those tests/assumptions aligned to reality) and then automatically run integration tests to catch unintended assumptions or regressions.

Also, I noticed we might have used a different (less optimized) approach with something like this:

const {Lambda} = require("@aws-sdk/client-lambda")
// this is an aggregated client that includes/imports all commands
const lambdaClient = new Lambda()

and then use it like this:

module.exports.createLambdaAlias = (lambdaARN, alias, version) => {
    const params = {
        FunctionName: lambdaARN,
        FunctionVersion: version,
        Name: alias,
    };
    return lambdaClient.createAlias(params));
};

which has an almost identical syntaxt as SDK v2 (just without the .promise():

return lambdaClient.createAlias(params).promise();

The main benefit would have been fewer code changes and no command classes to import. The obvious disadvantage is that you don't benefit from the SDKv3 modularity & performance.

I'm totally ok with the more optimized approach we have now 👌

@mettke
Copy link
Contributor Author

mettke commented Jan 9, 2024

Interesting. I did not see that this is also possible. To be honest, I did not work with v3 too much, I basically used the documentation to migrate one to the next and it shows you the other way to use it.
Even though I'm not sure how I feel about having Lambda reference the Class [1] and an Interface [2] 😅. Who uses the same name for two different structures...

[1] https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-lambda/Class/Lambda/
[2] https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-lambda/Interface/Lambda/

@alexcasalboni alexcasalboni merged commit 4a28bfc into alexcasalboni:master Jan 9, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants