SEZ9 commented on PR #11814:
URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5747123570
Thanks @goutamadwant for the update — good to hear all seven handler tests
pass on Java 11 locally.
**F4 / F6 (Kotlin & OkHttp pinning).** The pom trace earlier in this thread
(`kotlin-bom:1.9.10` imported into the engine-server module's
`dependencyManagement`, `okhttp` managed at the root via `${okhttp.version}`)
addresses the pinning concern. Since the resolved tree hasn't actually been
posted here yet, could you paste the `mvn dependency:tree
-Dincludes=org.jetbrains.kotlin,com.squareup.okhttp3` output against the
current head? If it confirms a uniform 1.9.10 graph, I'll consider F4 resolved,
and please align the license/dependency inventory entries that still list
1.8.21 so they match the actual dist.
**F1 (E2E wait strategy).** The call-order trace for the `startUp()` →
`createSeaTunnelServer(NETWORK)` path, where `executeExtraCommands(server)`
runs before `server.start()`, is convincing. Since
`createSeaTunnelContainerWithFakeSourceAndInMemorySink` has the opposite order,
could you either confirm `FakeSourceToConsoleWithEventReportIT` only ever goes
through the `startUp()` path, or add a short comment in the override noting
that assumption? With that I'm fine closing F1.
**Still open** (since the handler patch is unchanged, these haven't moved):
- **F2 / F3** – in the handler's close(): when the scheduler doesn't
terminate, the local buffer is still flushed concurrently with the interrupted
task, and the flush only catches
`HazelcastInstanceNotActiveException`/`IOException`. Please guard the flush on
successful termination (or drain under the same lock) and widen the catch so
other runtime failures don't escape close().
- **F5** – `testRetryAfterHttpFailure`: use the timeout variant of
takeRequest so the build can't hang, close the request Buffers, and make the
scheduler trigger explicit rather than relying on it firing despite the 1-day
interval.
- **F7** – please disable redirect following on the new OkHttpClient so
configured report headers/tokens aren't forwarded to a redirect target on
another host.
- **F8** – confirm whether `RequestBody.create(String, MediaType)` appending
`; charset=utf-8` to the Content-Type is acceptable for existing collectors, or
build the body from bytes to keep the header byte-identical.
I'll take another look once the queued CI run finishes.
<!-- streview-comment:1179 -->
--
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]
