Prefer Supplier<ProfileFile> over ProfileFileSupplier in APIs #3607
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cenedhryn
reviewed
Dec 9, 2022
dave-fn
force-pushed
the
davidfn/credentials-reload
branch
from
39b168f to
b39dc71
Compare
cenedhryn
reviewed
Dec 9, 2022
| @@ -63,6 +64,39 @@ void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFil | |||
| }); | |||
| } | |||
|
|
|||
| @Test | |||
| void resolveCredentials_requestFallsIntoProfileCredentialsProviderWithProfileFileSupplier_returnsCredentials() { | |||
There was a problem hiding this comment.
What does "requestFallsInto" mean? Is it even necessary in any of the test names?
There was a problem hiding this comment.
It refers to the provider within the chain doing the actual resolving. Will update test case names.
dave-fn
force-pushed
the
davidfn/credentials-reload
branch
from
b39dc71 to
468fce7
Compare
|
Kudos, SonarCloud Quality Gate passed! |
cenedhryn
approved these changes
Dec 12, 2022
dave-fn
added a commit
that referenced
this pull request
Feb 6, 2023
* ProfileCredentialsProvider can now reload credentials when profile files change (#3487) * ProfileFile can update if disk changed, reload as new instance * ProfileCredentialsProvider reloads credentials if profile file has changes * Created class ProfileFileRefresher * Created ReloadingProfileCredentialsProvider; moved new logic in ProfileFile to ProfileFileRefresher * Fix ReloadingProfileCredentialsBehavior when missing ProfileFile Supplier or Predicate, and dealing with defaults * Consolidated ReloadingProfileCredentialsProvider functionality into ProfileCredentialsProvider * Fix behavior when dealing with defaults * Created ProfileFileSupplier interface; refactored * Misc fixes * Created package-private ProfileFileSupplierBuilder; ProfileFileSupplier now extends Supplier; Fixed Javadoc * Fixed unit tests for default credentials file * Removed ProfileFileSupplier.Builder interface * Code cleanup * ProfileFileSupplier API changes, aggregating (#3558) * Added methods for aggregating ProfileFile objects * Removed redundant logic, changelog entry * Removed redundant methods * Use compare and set to make thread safe * DefaultCredentialsProvider reload profile (#3580) * Misc fixes * Reload credentials by DefaultCredentialsProvider; pass supplier to InstanceProfileCredentialsProvider * Fix code alignment * Update public APIs to Supplier<ProfileFile> instead of ProfileFileSupplier (#3607) * ProfileTokenProvider Reload ProfileFile (#3608) * Updated ProfileTokenProvider * Updated tests, do not explicitly swallow exceptions * ProfileTokenProviderLoader can use Supplier<Profilefile> internally * Simplified ProfileTokenProviderLoader API; implemented synchronized block * Use synchronized block (#3646) * S3 support classes take Supplier<ProfileFile> (#3653) * S3 support classes take Supplier<ProfileFile> * Review comments * Presigners, other Support classes take Supplier<ProfileFile> (#3677) * Presigners, other Support classes take Supplier<ProfileFile> * Split new ProfileFile tests from existing parameterized tests * Improved tests readability * ProfileFile updates to BaseClientBuilderClass, other S3 classes (#3685) * Leverage SdkClientOption and SdkExecutionAttributes; fallback to simp… (#3692) * Leverage SdkClientOption and SdkExecutionAttributes; fallback to simple ProfileFile * Addressed review comments * Updated changelog entry (#3699) * Fixed review comments * Removed unnecessary logic * Deprecated SdkClientOption PROFILE_FILE * Deprecated SdkExecutionAttribute PROFILE_FILE * Updated changelog entry
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
In order to keep the APIs consistent, current public methods taking
ProfileFileSupplieras an argument will be switched over toSupplier<ProfileFile>. The former class is a subclass of the latter, so this is not a breaking change.Testing
Only existing test cases.
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License