-
Notifications
You must be signed in to change notification settings - Fork 896
Add client configuration overriding of SCHEDULED_EXECUTOR_SERVICE option #4002
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!! Looks great, we just need to make sure the custom ScheduledExecutorService doesn't get closed when the client is closed. Also a few minor comments.
...-core/src/test/java/software/amazon/awssdk/core/client/builder/DefaultClientBuilderTest.java
Outdated
Show resolved
Hide resolved
...k-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java
Show resolved
Hide resolved
...-core/src/test/java/software/amazon/awssdk/core/client/builder/DefaultClientBuilderTest.java
Outdated
Show resolved
Hide resolved
...ore/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java
Show resolved
Hide resolved
Should I add more commits to the PR or force pushed ? |
You can add more commits to the PR |
...k-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java
Show resolved
Hide resolved
...ore/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java
Show resolved
Hide resolved
SonarCloud Quality Gate failed. |
@all-contributors please add @scrocquesel for code. |
@scrocquesel Great stuff! PR has been merged 👍 |
Trying again :) And adding contributors from previous PRs. please add @scrocquesel for code. |
I've put up a pull request to add @scrocquesel! 🎉 I've put up a pull request to add @dave-fn! 🎉 I've put up a pull request to add @L-Applin! 🎉 |
Motivation and Context
Customer should be able to use their own ThreadPool and reuse it accros multiple clients.
#1690
Modifications
Introduce new client config methods to allow to override
SCHEDULED_EXECUTOR_SERVICE
option. It is optional and when not provided will default to the actual code that create a new ThreadPool for each clientsTesting
Unit tests ensure the override is applied on the client configuration.
I also tested in a private project by setting api call timeout to actually create new thread on the custom thread pool.
Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsscripts/new-change
script and following the instructions. Commit the new file created by the script in.changes/next-release
with your changes.License