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

Get PrestoS3FileSystem to work with the AWS Default Credentials Provider #13858

Merged
merged 1 commit into from Dec 26, 2019

Conversation

anoopj
Copy link
Contributor

@anoopj anoopj commented Dec 12, 2019

DefaultAWSCredentialsProviderChain is frequently used by AWS customers
and it provides access from a documented list of sources. This
especially makes it easier to run Presto on non-EC2 hosts where you
don't have the instance profile. (e.g. Macs, during development).

See:
https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.html

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* Get PrestoS3FileSystem to work with the AWS Default Credentials Provider

Copy link
Contributor

@ajaygeorge ajaygeorge left a comment

Choose a reason for hiding this comment

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

LGTM with a minor comment

throws Exception
{
Configuration config = new Configuration();
config.setBoolean(S3_USE_INSTANCE_CREDENTIALS, false);

try (PrestoS3FileSystem fs = new PrestoS3FileSystem()) {
fs.initialize(new URI("s3n://test-bucket/"), config);
assertInstanceOf(getAwsCredentialsProvider(fs), DefaultAWSCredentialsProviderChain.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like you could an
assertEquals(getAwsCredentialsProvider(fs), DefaultAWSCredentialsProviderChain.getInstance())
which is better than assertInstanceOf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recommend keeping this, since this is more descriptive and the DefaultAWSCredentialsProviderChain class is a stronger contract than the instance.

@ajaygeorge
Copy link
Contributor

ajaygeorge commented Dec 17, 2019

@anoopj Integrate PrestoS3FileSystem with AWS Default Credentials Provider reads better as the commit message headline?

@anoopj
Copy link
Contributor Author

anoopj commented Dec 17, 2019

@anoopj Integrate PrestoS3FileSystem with AWS Default Credentials Provider reads better as the commit message headline?

@ajaygeorge I will change the commit message headline.

DefaultAWSCredentialsProviderChain is frequently used by AWS customers
and it provides access from a documented list of sources. This
especially makes it easier to run Presto on non-EC2 hosts where you
don't have the instance profile. (e.g. Macs, during development).

See:
https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.html
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.

None yet

3 participants