Skip to content

update calls to aws.request to use new API (fixes #41)#42

Merged
fernando-mc merged 1 commit intofernando-mc:masterfrom
linusmarco:issue-41
Mar 12, 2018
Merged

update calls to aws.request to use new API (fixes #41)#42
fernando-mc merged 1 commit intofernando-mc:masterfrom
linusmarco:issue-41

Conversation

@linusmarco
Copy link
Copy Markdown
Contributor

Background

As described in #41, our calls to the AWS SDK request method use the old API, which has arguments for region and stage. The new API does not support these arguments, instead storing the region and stage elsewhere. Instead there is an optional options argument (which we don't need for anything in serverless-finch at the moment). Region and stage are set at initialization.

Proposed changes

Change all calls to aws.request to remove the region and stage arguments. Example:

return this.aws.request('S3', 'putObject', params, this.stage, this.region);
return this.aws.request('S3', 'putObject', params);

@fernando-mc
Copy link
Copy Markdown
Owner

@linusmarco I'd be super surprised if this doesn't break anything. Have you already tested it?

@linusmarco
Copy link
Copy Markdown
Contributor Author

@fernando-mc, yup I tested it and everything works. The way the updates Serverless API works, the region and stage are taken from serverless.yml on initialization of the Serverless class. You can check out the signature of the request function here:
https://github.com/serverless/serverless/blob/master/lib/plugins/aws/provider/awsProvider.js#L196

Right now, we're passing stage in the options argument, which ends up getting ignored/overridden on line 200 in the file linked above.

Someone else should also test to make sure, though

@fernando-mc fernando-mc merged commit 2ec6c0f into fernando-mc:master Mar 12, 2018
@fernando-mc
Copy link
Copy Markdown
Owner

@linusmarco tested. This works.

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