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

Provide AWS_PROFILE from configuration for invoke local #5662

Merged
merged 3 commits into from Jan 22, 2019

Conversation

Projects
3 participants
@revmischa
Copy link
Contributor

revmischa commented Jan 8, 2019

What did you implement:

Closes #5661

How did you implement it:

Exactly like the AWS_REGION environment code.

How can we verify it:

Create two profiles in .aws/config, make profile1 default
Set provider.profile to profile2
Run invoke - see that it runs as profile2
Run invoke local - see that it runs as profile1

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?: NO
Is it a breaking change?: NO

@pmuens pmuens self-assigned this Jan 16, 2019

@pmuens
Copy link
Member

pmuens left a comment

Thanks for working on this @revmischa 👍

This looks good from a code perspective! Could you add unit tests so that we got it test covered?

Let us know if you have any questions / need any help here!

@pmuens pmuens added this to In progress in Serverless via automation Jan 16, 2019

@pmuens pmuens moved this from In progress to Needs review in Serverless Jan 16, 2019

@revmischa

This comment has been minimized.

Copy link
Contributor Author

revmischa commented Jan 16, 2019

@pmuens okay i tried.. but failed. can you please review my test and help me figure out what it should look like? thanks

@pmuens

pmuens approved these changes Jan 22, 2019

Copy link
Member

pmuens left a comment

Hey @revmischa thanks again for taking the time to work on this and following up on the PR review comments 👍

I just looked into this and the issue you ran into had something todo with out hook system and the state it creates so your test was correct, however there was a need to run the hooks (described at the top of the invokeLocal plugin) to get the data in the correct shape (we run extendedValidate in the hooks which modifies options). I updated the tests and simplified them so that they don't require the hooks to be run beforehand.

Futhermore I added some tests for getProfile. I pushed my changes on top of your branch / PR.

Given that it's GTM and I'll merge the PR as soon as the tests are green. :shipit: 👌

Serverless automation moved this from Needs review to Reviewer approved Jan 22, 2019

@pmuens pmuens added pr/in-review and removed pr/in-progress labels Jan 22, 2019

@revmischa

This comment has been minimized.

Copy link
Contributor

revmischa commented on da83822 Jan 22, 2019

Awesome thanks!

@pmuens pmuens merged commit 89d037c into serverless:master Jan 22, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+2.2%) to 93.018%
Details

Serverless automation moved this from Reviewer approved to Done Jan 22, 2019

@shortjared shortjared added this to the v1.36.3 milestone Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment