-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Another prototype for async HttpClient APIs #45836
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
base: main
Are you sure you want to change the base?
Another prototype for async HttpClient APIs #45836
Conversation
sdk/clientcore/http-okhttp3/src/main/java/io/clientcore/http/okhttp3/OkHttpHttpClient.java
Outdated
Show resolved
Hide resolved
...ientcore/core/src/main/java/io/clientcore/core/implementation/http/client/JdkHttpClient.java
Outdated
Show resolved
Hide resolved
sdk/clientcore/core/src/main/java/io/clientcore/core/http/client/HttpClient.java
Show resolved
Hide resolved
sdk/clientcore/core/src/main/java/io/clientcore/core/http/client/HttpClient.java
Show resolved
Hide resolved
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
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.
looking good - a few minor comments and some cleanup.
sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpPipeline.java
Outdated
Show resolved
Hide resolved
sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpRedirectPolicy.java
Outdated
Show resolved
Hide resolved
* used for re-authentication. | ||
* | ||
* <p> | ||
* The default implementation doesn't handle challenges. You can override and your implementation as needed. |
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.
I'm not convinced we want to default to a bad implementation? If we can do anything useful, it's probably better to do nothing at all?
* The default implementation doesn't handle challenges. You can override and your implementation as needed. | |
* The default implementation doesn't handle challenges. You can override in your implementation as needed. |
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.
I'll leave this question to @g2vinay as I was simply making the async code path based on the sync logic.
Description
Related to #45816
This is another prototype for an async HttpClient send method. This design has the async API declared in
HttpClient
assendAsync
with a default implementation.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines