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

Fix 2404 Receive "Deployment bucket is not in the same region..." when using eu-west-1 #3107

Merged
merged 3 commits into from Jan 18, 2017

Conversation

ssemyonov
Copy link

@ssemyonov ssemyonov commented Jan 16, 2017

What did you implement:

Closes #2404

How did you implement it:

Similar to original #2404 fix for us-east-1:
if (result.LocationConstraint === 'EU') result.LocationConstraint = 'eu-west-1';

How can we verify it:

By deploying using pre-existing bucket in eu-west-1 region in one of the affected AWS accounts (which might be tricky to find).

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config/commands/resources
  • Enable "Allow edits from maintainers" for this PR
  • Change ready for review message below

Is this ready for review?: YES
Is it a breaking change?: NO

Copy link
Contributor

@pmuens pmuens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ssemyonov thank you for this PR! 👍

I tried to reproduce #2404 but was unsuccessful to do so. I checked out your PR and was able to deploy to eu-west-1 w/o problems.

Don't know if @eahefnawy can reproduce this.

I'm fine with merging it since it seems to resolve the issue and is one line of code which won't break anything. Although unfortunately I cannot confirm that it fixes the issue mentioned in #2404.


awsPlugin.options.region = value.region;

sinon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stub should be restored.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awsPlugin.provider.request.restore(); added below

@eahefnawy
Copy link
Member

Yeah this is hard to reproduce as it depends on what AWS might return. Considering the change is a simple if statement, I see no harm in merging without testing it out just like @pmuens mentioned.

Copy link
Member

@eahefnawy eahefnawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Once @pmuens sinon comment is resolved we can merge!

@pmuens
Copy link
Contributor

pmuens commented Jan 18, 2017

@ssemyonov thanks for the update! Merging...

@pmuens pmuens merged commit 158b1bf into serverless:master Jan 18, 2017
@ssemyonov ssemyonov deleted the fix-2404 branch January 18, 2017 13:19
@ssemyonov
Copy link
Author

cheers

@nick4eva
Copy link

@pmuens I assume this fix will be released in Serverless 1.6? Don't see it in the milestone 1.6.

@pmuens
Copy link
Contributor

pmuens commented Jan 19, 2017

@nick4eva Yes, this one will be released in 1.6 (maybe even in 1.5.1 which is planned for the end of this week)! Will add it to the 1.6 milestone...

@pmuens pmuens added this to the 1.6 milestone Jan 19, 2017
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.

Receive "Deployment bucket is not in the same region as the lambda function" when using us-east-1
4 participants