Skip to content
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

Align HttpRequester/StreamingHttpRequester with blocking variants #3201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

BlockingHttpRequester and BlockingStreamingHttpRequester implement GracefulAutoCloseable interface and in result can be used in try-with-resources. For some reason, our HttpRequester and StreamingHttpRequester don't but their client and connection interfaces do.

Modifications:

  • Promote GracefulAutoCloseable interface from HttpClient and HttpConnection to HttpRequester.
  • Promote GracefulAutoCloseable interface from StreamingHttpClient and StreamingHttpConnection to StreamingHttpRequester.

Result:

All requester/client/connection interfaces are consistent across all 4 API variants.

Risk:

Minimal, japicmp.sh output:

Comparing binary compatibility of servicetalk-http-api-0.42.55-SNAPSHOT.jar against servicetalk-http-api-0.42.54.jar
No changes.

However, use-cases that have a static import of a method with name close on a requester implementation won't compile. See changes in H2ClientParentConnectionContext. Error:

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/H2ClientParentConnectionContext.java:394: error: method close in interface StreamingHttpRequester cannot be applied to given types;
                              close(streamChannel, cause);
                              ^
    required: no arguments
    found: Http2StreamChannel,Throwable
    reason: actual and formal argument lists differ in length

I consider that as a minimal risk, and ready to revert this change in future releases if any users find it problematic.

Motivation:

`BlockingHttpRequester` and `BlockingStreamingHttpRequester` implement
`GracefulAutoCloseable` interface and in result can be used in
try-with-resources. For some reason, our `HttpRequester` and
`StreamingHttpRequester` don't but their client and connection
interfaces do.

Modifications:

- Promote `GracefulAutoCloseable` interface from `HttpClient`
and `HttpConnection` to `HttpRequester`.
- Promote `GracefulAutoCloseable` interface from `StreamingHttpClient`
and `StreamingHttpConnection` to `StreamingHttpRequester`.

Result:

All requester/client/connection interfaces are consistent across all 4
API variants.
@idelpivnitskiy idelpivnitskiy requested review from daschl and mgodave March 1, 2025 00:22
@idelpivnitskiy idelpivnitskiy self-assigned this Mar 1, 2025
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.

1 participant