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

Added Support to fetch aws region from aws config file .aws/config #537 #539

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

namanvats
Copy link

@namanvats namanvats commented Jun 16, 2024

What does this PR do?
This PR adds support to fetch the AWS region name from the ~/.aws/config file and use the environment variable as a fallback if the aws config is absent. It defaults to us-east-1 in case both of them are not present. It leverages boto3 which is already present in requirements. So no changes to requirements are required.

Tries to fix the issue raised here : #537

@namanvats namanvats requested a review from a team as a code owner June 16, 2024 13:35
@namanvats namanvats changed the title Added Support to fetch aws region from aws config file .aws/config Added Support to fetch aws region from aws config file .aws/config #537 Jun 16, 2024
Copy link
Contributor

@JDKardia JDKardia left a comment

Choose a reason for hiding this comment

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

I like this, just needs a tweak!

@@ -116,6 +117,8 @@ def __init__(

self.aws_access_key = aws_access_key

session = boto3.Session()
aws_region = session.region_name
Copy link
Contributor

Choose a reason for hiding this comment

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

This is almost what we want! Ideally we'd want to set aws_region in order of specificity:

  1. specific region passed as argument
  2. specific region passed as environment variable
  3. specific region within config
  4. default to us-east-1

however, as written in this PR, this would be:

  1. specific region within config
  2. specific region passed as argument
  3. specific region passed as environment variable
  4. default to us-east-1

if you could correct the ordering of assignment, that would be wonderful.

Choose a reason for hiding this comment

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

@JDKardia, I fixed the issue here at #669, please review and merge

@rattrayalex
Copy link
Collaborator

Thank you for the PR. We hope to take a look soon.

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.

4 participants