Skip to content

Fix aws-profile cli option to override env & config#3980

Merged
pmuens merged 2 commits into
serverless:masterfrom
mpuittinen:master
Aug 23, 2017
Merged

Fix aws-profile cli option to override env & config#3980
pmuens merged 2 commits into
serverless:masterfrom
mpuittinen:master

Conversation

@mpuittinen

Copy link
Copy Markdown
Contributor

What did you implement:

Closes #3979

How did you implement it:

Split handling of serverless.provider.profile (from serverless.yml) and cli option aws-profile into two different cases in awsProvider.js since they have different prioritites.

Set the AWS_PROFILE env in the aws-profile option test to verify that the profile set overrides the env.

How can we verify it:

AWS_PROFILE=foobar serverless deploy --aws-profile myprofile

This now deploys to myprofile, previously it tried to deploy to foobar.

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
  • Update the messages below

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

@eahefnawy

Copy link
Copy Markdown
Contributor

@mpuittinen this looks great. Wanna merge it in, but seems like there's something wrong with Github/Travis integration. The build is passing, but Github is reporting an error, not sure why. Maybe it needs a refresh of some kind.

Do you mind maybe updating your branch to refresh the build? maybe a pull from our master would suffice?

@pmuens pmuens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested this as well. Works on my machine...

Looks like the build problem was resolved so we can merge this 🎉 --> Merging... :shipit:

@pmuens pmuens merged commit 21dd400 into serverless:master Aug 23, 2017
@pmuens pmuens added this to the 1.21 milestone Aug 23, 2017
@eahefnawy eahefnawy mentioned this pull request Aug 30, 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.

3 participants