FrankChen021 commented on code in PR #19607:
URL: https://github.com/apache/druid/pull/19607#discussion_r3459585609
##########
processing/src/main/java/org/apache/druid/java/util/http/client/response/HttpResponseHandler.java:
##########
@@ -98,5 +98,13 @@ interface TrafficCop
* @return time that backpressure was applied (channel was closed for
reads)
*/
long resume(long chunkNum);
+
+ /**
+ * Closes the underlying connection, abandoning any remaining response.
+ *
+ * Intended for callers that decide they no longer need the rest of the
response (for example, because the
+ * consumer of the resulting stream has been closed early)
+ */
+ void abort();
Review Comment:
Thanks for clarifying. I agree Netty should define the actual close behavior
explicitly, and it does here. My concern is narrower: TrafficCop is a public
nested interface that was previously SAM-compatible, so adding abort() as a
required abstract method breaks source compatibility for lambdas/custom
HttpClient fakes and can still throw AbstractMethodError for compiled
implementations if this path is called. A default no-op does not stop Netty
from overriding abort(); it just preserves the existing contract for
implementations that cannot abort. If the intent is to require all
implementations to make an explicit choice, I think this should be treated as
an intentional API break and reflected accordingly. Reviewed 7 of 7 changed
files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
