SEZ9 commented on PR #11814: URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5806560939
Thanks for the detailed per-item walkthrough against `ec6a7f9affc`. The descriptions for F1 (base fixture log-message wait remaining the readiness gate, with the wrapper only adding thread diagnostics on a failed wait), F2/F3 (`CompletionException` unwrapping, INFO/ERROR split, flush regardless of branch, unconditional `cancelAll()`/`evictAll()` and interrupt-flag restoration in `finally`), F4/F6 (`kotlin-bom:1.9.10` import, unversioned `kotlin-stdlib-jdk8`, and both inventories listing only `1.9.10`), and F5 (bounded `takeRequest(10, TimeUnit.SECONDS)`, try-with-resources on both `Buffer`s, explicit `handler.report()` calls) all line up with what I asked for. I will confirm each against the actual diff before marking them resolved rather than closing them on the summary alone. Your F5 note appears to have been cut off mid-sentence; could you finish the point about the 1-day interval so I have the full picture? I also have not yet independently checked that `8f4dce57d45`, `af9b89d9e`, and `ec6a7f9affc` leave this PR's own files untouched since `73f66f922`; I will do that as part of the same pass. Two items from my earlier review are not covered in your update: 1. **F7 (redirects):** Does the `OkHttpClient` still follow redirects by default? If so, please explain why forwarding the configured report headers to a redirect target is acceptable, or disable following redirects. 2. **F8 (Content-Type charset):** How is the request body built? Please confirm the configured media type is sent byte-identical without an appended `; charset=utf-8`, or note that the header change is intended. A short reply on F5, F7, and F8 is all I need to finish the re-review. <!-- streview-comment:1274 --> -- 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]
