shoemoney commented on PR #20151:
URL: https://github.com/apache/druid/pull/20151#issuecomment-5641989008

   Ported onto `51e31a68c2`. Pushed `902448131e`.
   
   The detection moved rather than merged. `handleResponse` now records only 
the status and Content-Type, and the non-JSON prefix check runs in 
`handleChunk` against the first chunk carrying a non-whitespace byte. That call 
site already existed for chunked replies, so it is now the only one and the 
code is simpler than before. `ChannelBuffer` became `ByteBuf` in 
`bodyPrefixByte`, `failIfNonJsonBody` and `throwForNonJsonBody`; all three use 
`readerIndex`/`readableBytes`/`getByte`, which are identical on Netty 4.
   
   I dropped my `NettyHttpClient` change in favour of yours. #19567 solved the 
same problem with `if (!retVal.isDone()) retVal.setException(ex)` plus a 
rethrow, and carrying two spellings of one fix would be worse than taking the 
one that is already in. I kept `NettyHttpClientTest` and ported it, because 
#19567 added no test there and both of its cases assert the behaviour rather 
than the mechanism. They pass unmodified against your implementation, which is 
the useful part: they now lock in the new code rather than mine.
   
   `DirectDruidClientTest`'s fake client no longer puts `bodies[0]` on the 
response. Every element is delivered as an `HttpContent` and `chunkNum` starts 
at 0, matching what Netty 4 actually does.
   
   Verified locally with `mvn -pl server -am ... test` on JDK 26:
   
   ```
   DirectDruidClientTest   22 passed, 0 failures   (upstream/master baseline: 
12)
   JsonParserIteratorTest  16 passed, 0 failures
   NettyHttpClientTest      2 passed, 0 failures
   ```
   
   The baseline was measured in a separate worktree at `51e31a68c2` with the 
identical invocation. Worth recording one wrong turn: an earlier run using 
`surefire:test` instead of `test` reported 14 failures on my branch and a fully 
red `DirectDruidClientTest` on clean master. That was a stale 
`druid-processing` jar being resolved from `~/.m2` instead of the reactor 
build. Master is fine.
   


-- 
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]

Reply via email to