8000 Give precedence to region override over config by viren-nadkarni · Pull Request #8997 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Give precedence to region override over config #8997

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 31, 2023
Merged

Give precedence to region override over config #8997

merged 3 commits into from
Aug 31, 2023

Conversation

viren-nadkarni
Copy link
Member

Motivation

The Boto client has an odd behaviour where

  • if config region is non-default and arg region is also non-default, it uses the arg region
  • but when config region is non-default and arg region is 'us-east-1', it uses the config region

This is better illustrated below:

In [29]: session = botocore.session.Session()

In [30]: config = botocore.config.Config(region_name='us-west-1')

In [32]: session.create_client(service_name='s3', region_name='us-east-1', config=config).meta.region_name
Out[32]: 'us-west-1'

In [33]: session.create_client(service_name='s3', region_name='ap-south-1', config=config).meta.region_name
Out[33]: 'ap-south-1'

This PR handles this behaviour in our internal client library and makes things consistent.

@viren-nadkarni viren-nadkarni self-assigned this Aug 28, 2023
@viren-nadkarni viren-nadkarni added the semver: patch Non-breaking changes which can be included in patch releases label Aug 28, 2023
@viren-nadkarni viren-nadkarni requested a review from dfangl August 28, 2023 13:57
@viren-nadkarni viren-nadkarni marked this pull request as ready for review August 28, 2023 13:57
@viren-nadkarni viren-nadkarni requested a review from thrau as a code owner August 28, 2023 13:57
@github-actions
Copy link
github-actions bot commented Aug 28, 2023 •
edited
Loading

LocalStack Community integration with Pro

       2 files         2 suites   1h 28m 44s ⏱️
2 149 tests 1 671 ✔️ 478 💤 0 ❌
2 150 runs  1 671 ✔️ 479 💤 0 ❌

Results for commit a898079.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

Coverage Status

coverage: 80.758% (+0.02%) from 80.743% when pulling a898079 on region-override into 601aecb on master.

@viren-nadkarni viren-nadkarni mentioned this pull request Aug 30, 2023
Closed
@viren-nadkarni viren-nadkarni merged commit 4548bf4 into master Aug 31, 2023
@viren-nadkarni viren-nadkarni deleted the region-override branch August 31, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfangl dfangl dfangl approved these changes

@thrau thrau Awaiting requested review from thrau thrau is a code owner

Assignees

@viren-nadkarni viren-nadkarni

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@viren-nadkarni @coveralls @dfangl
0