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

Setting aws session region from config #45

Merged
merged 2 commits into from
Dec 10, 2019
Merged

Conversation

joshmarsh
Copy link
Contributor

@joshmarsh joshmarsh commented Dec 3, 2019

Proposed changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (changes to code, which do not change application behavior)

Checklist

  • I have filled out this PR template
  • I have read the CONTRIBUTING doc
  • I have added automated tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (README.md, inline comments, etc.)
  • I have updated the CHANGELOG.md under a ## next release, with a short summary of my changes

Dependencies and Blockers

Relevant Links

Further comments

@@ -35,6 +35,7 @@ func New(config *configs.Root, configFile string, observation *observ.Observatio
if err != nil {
log.Fatalf("Failed to initialize AWS Session: %s", err)
}
awsSession.Config.Region = config.Region
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The shared credentials provider seems to be loading credentials from ~/.aws, but not the region. I also don't see anything in the code suggesting that it is loading the region.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it might not be configured the regions from the credentials file.

There's actually a few other places, though, where we set this default to us-east-1. I'd rather this isn't scattered all over the code base. Can we get it centralized in a single spot somewhere?

eg

if s.Config.Region == nil {

if input.region == nil {

@eschwartz eschwartz merged commit abfa706 into master Dec 10, 2019
@eschwartz eschwartz deleted the bug/deploy-region-not-set branch December 10, 2019 17:30
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