Remove default aws region #318
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will still allow the region to be specified via '-r', but adds the
additional option to set it via `AWS_DEFAULT_ENVIRONMENT`.
A failure to provide either will generate a fairly clear stacktrace
like:
```
...
File "/Users/troyready/Library/Python/2.7/lib/python/site-packages/botocore/regions.py", line 135, in _endpoint_for_partition
raise NoRegionError()
botocore.exceptions.NoRegionError: You must specify a region.
```
|
As an aside: I ended up writing this after taking a more serious look at fixing #277 and finding the required changed to be so daunting that it seemed better to just keep putting it off. |
|
Looks good to me, I like that it doesn't just automatically assume the default region. @troyready would you be able to write some tests for it? |
|
@ttaub sure thing, PR updated. The only change to the connection code is logic around |
troyready
force-pushed
the
remove_default_region
branch
from
16e1cef
to
971a941
Compare
|
Honestly, I think we should just do away with --region altogether, and use the standard (ie: either profile, the environment variable, or stack specific - when we have stack specific). What do you think @troyready ? |
phobologic
approved these changes
Feb 7, 2017
phrohdoh
pushed a commit
to phrohdoh/stacker
that referenced
this pull request
Dec 18, 2018
Remove default aws region
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will still allow the region to be specified via '-r', but adds the
additional option to set it via
AWS_DEFAULT_ENVIRONMENT.A failure to provide either will generate a fairly clear stacktrace
like:
This does introduce an additional unit testing requirement to set the environment variable; not sure if there's a better way to handle that for the kms lookup?
It's a fairly major change, but this follows alongside the logic of #298 and will help keep mistakes from happening (a NoRegionError is more clear than an xref failing with a output lookup stacktrace, or a bunch of stuff deployed to the wrong region).