Skip to content

aws credentials cleanup #568

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

Merged
merged 1 commit into from
Jun 22, 2018
Merged

aws credentials cleanup #568

merged 1 commit into from
Jun 22, 2018

Conversation

zoewangg
Copy link
Contributor

@zoewangg zoewangg commented Jun 22, 2018

Currently, AwsSessionCredentials extends AwsCredentials, but it's really not a is-a relation and the inheritance has many issues/limitations, eg: inherited static factory methods.

  • Updated AwsCredentials to interface
    • AwsSessionCredentials implements AwsCredentials
    • AwsBasicCredentials implements AwsCredentials

AwsBasicCredentials class is named BasicAwsCredentials in v1. I named it this way to match AwsSessionCredentials, but this is open to discussion.

  • renamed getCredentials() to resolveCredentials() as we discussed.

will create changelog entry before merge


@Override
public int hashCode() {
return Objects.hash(accessKeyId, secretAccessKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but we should prefer Objects.hashCode since Objects#hash needs to create a temporary array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, sure, good call, was not aware of it. I'll fix it and it might be a be a good idea to add a checkstyle for this.

Copy link
Contributor

@dagnir dagnir left a comment

Choose a reason for hiding this comment

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

:shipit:

@dagnir
Copy link
Contributor

dagnir commented Jun 22, 2018

Thanks for adding a checkstyle rule!

…ls and AwsSessionCredentials. Rename getCredentials to resolveCredentials
@zoewangg zoewangg force-pushed the zoewang-awsSessionCredentials branch from 461083f to 864a6bd Compare June 22, 2018 19:27
@zoewangg zoewangg merged commit e93f48d into master Jun 22, 2018
@zoewangg zoewangg deleted the zoewang-awsSessionCredentials branch June 22, 2018 19:50
aws-sdk-java-automation added a commit that referenced this pull request Jul 24, 2019
…d6e3f28b

Pull request: release <- staging/d55fe5a4-0306-43a3-8289-8764d6e3f28b
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