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

S3 crt client doesn't pick up the default region #1768

Closed
dhananjays opened this issue Sep 9, 2021 · 6 comments
Closed

S3 crt client doesn't pick up the default region #1768

dhananjays opened this issue Sep 9, 2021 · 6 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@dhananjays
Copy link

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
S3-crt client doesn't automatically get the region unlike the S3 client which automatically fetches region (using the chain of AWS_DEFAULT_REGION -> profile -> instance metadata). The user is forced to provide the correct region when creating the s3-crt client

SDK version number
1.9

Platform/OS/Hardware/Device
EC2 Linux instance

To Reproduce (observed behavior)
Steps to reproduce the behavior (please share code)
Create S3 client without setting region in the client config. It automatically picks up the default region from the profile/instance metadata (This is as expected from version 1.8 as mentioned here - https://aws.amazon.com/blogs/developer/aws-sdk-for-c-version-1-8-developer-preview/)

Create S3-crt client without setting region in the client config. It does not pick the default region as expected.

Expected behavior
S3-crt client should automatically pick up the default region as done by the S3 client

@dhananjays dhananjays added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2021
@KaibaLopez
Copy link
Contributor

KaibaLopez commented Sep 9, 2021

Hi @dhananjays ,
So, it seems like you've figured out the workaround , but for the record this behavior is not expected of the s3-crt default constructor.

The s3-crt default provider chain is reduced to make it faster to go through, and it will only check for the region in the AWS_DEFAULT_REGION environment variable, a specified profile in the config file or the one hard coded on the Aws::S3Crt::ClientConfiguration object.
Having said that, this could be a feature request, but I can't make any promises on when we could work on it.

@KaibaLopez KaibaLopez added feature-request A feature should be added or improved. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2021
@grrtrr
Copy link
Contributor

grrtrr commented Mar 19, 2022

Here is how we back-ported that in our code, since we ran into the same issue: the CRT does not pickup AWS_{,DEFAULT_}REGION:

        Aws::S3Crt::ClientConfiguration config;
        // ...
        config.region = []() -> Aws::String {
            // Return the configured AWS region, using us-east-1 as fallback. This is normally part
            // of Aws::Client::ClientConfiguration, but is not implemented for the S3CrtClient.
            Aws::String region = os::getenv("AWS_DEFAULT_REGION");
            if (!region.empty()) {
                return region;
            }

            region = os::getenv("AWS_REGION");
            if (!region.empty()) {
                return region;
            }

            // The profile may also contain a region value.
            if (Aws::Config::HasCachedCredentialsProfile("default")) {
                region = Aws::Config::GetCachedCredentialsProfile("default").GetRegion();
                if (!region.empty()) {
                    return region;
                }
            }
            return Aws::Region::US_EAST_1;
        }();

@grrtrr
Copy link
Contributor

grrtrr commented Mar 19, 2022

The problem is also resolved by #1884.

@jmklix
Copy link
Member

jmklix commented May 3, 2024

Merged: #2098

@jmklix jmklix closed this as completed May 3, 2024
Copy link

github-actions bot commented May 3, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@grrtrr
Copy link
Contributor

grrtrr commented May 3, 2024

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

4 participants