-
Notifications
You must be signed in to change notification settings - Fork 910
Release CRT Sync HTTP client #4750
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
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
* implemented synchronous crt java http client with tests. * Forgot about informing the loader of what we did. * Still not completed with PR, but the large refactors have been done and I want to go ahead and get a review. Still WIP on moving the anonymous classes to another class. * Addressed refactor comments. Going to make another pass at adding some more integ tests. * make the client base abstract to force users to override it, since that's its intention. * Fixed but in the response stream handler for the async to sync stream adapter to properly complete and close upon end of the response. Began adding some tests but they don't all pass yet. * Updates to use new http exception error info for tls negotiation, changes to test running. passing sync client tests. * Revert to using the error code. Updated crt version to include the needed changes. * Refactored benchmarks and s3 stability tests to handle both async and sync runs. Added sync variants for crt clients for the http client benchmarks and s3 stability test. * Update the console output to have accurate test name. * Further code review fixes as well as updates for the linters. * Fixed some checkstyle errors I hope? * More checkstyle fixes. * PR Feedback * add volatile * Close threadpool at end of CRT tests --------- Co-authored-by: Waqar Ahmed Khan <[email protected]> Co-authored-by: Dongie Agnir <[email protected]>
* Update javadoc, minor refactoring * Fix tests * Revert checkstyle update
* Various performance optimization for CRT sync HTTP client * Return as soon as the stream starts if there is a payload in sync http client
dagnir
approved these changes
Dec 14, 2023
3f150ab
to
24b5bba
Compare
…e it increases test time and makes tests flaky especially when tests are running in parallel. We still verify it in wiremock tests and stability tests
23dea4a
to
bfc589e
Compare
|
dagnir
approved these changes
Dec 15, 2023
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
Merging CRT Sync HTTP client.
#3343
Testing
Running test suites
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