SEZ9 commented on PR #11814:
URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5738798275
Thanks for the follow-up on `5cde0fdd8` — the pom-based reasoning for F4 and
the call-order trace for F1 are both helpful.
**F4 (Kotlin/OkHttp pinning).** Agreed that `kotlin-bom:1.9.10` imported
into `seatunnel-engine/seatunnel-engine-server/pom.xml` `dependencyManagement`
(lines 30-38), together with the versionless `kotlin-stdlib-jdk8` at lines
112-115 and the root-managed `okhttp` (`pom.xml` lines 349-354,
`${okhttp.version}`), pins both by construction on this module's classpath. Two
remaining asks:
- Yes, please paste the `mvn dependency:tree
-Dincludes=org.jetbrains.kotlin,com.squareup.okhttp3` output for this head so
we have the resolved tree on record, not just the pom evidence.
- Since the runtime graph resolves to 1.9.10,
`tools/dependencies/known-dependencies.txt` and
`seatunnel-dist/release-docs/LICENSE` should not still carry a 1.8.21 / 1.9.10
split (F6). Please align the inventories with the actual resolved dist and
confirm the dependency check passes.
**F1 (wait strategy gating).** The trace makes sense: on the
`SeaTunnelContainer.startUp()` → `createSeaTunnelServer(NETWORK)` path,
`executeExtraCommands(server)` at line 160 runs before `server.start()` at line
162, so `container.waitingFor(...)` from
`FakeSourceToConsoleWithEventReportIT.executeExtraCommands` (lines 106-117) is
in effect when `.start()` blocks. I'm fine dropping the HIGH severity on that
basis. Your comment was cut off at the
`createSeaTunnelContainerWithFakeSourceAndInMemorySink` overload (lines
200-244), where `server.start()` at line 232 precedes `executeExtraCommands` at
line 234 — could you finish that thought and confirm
`FakeSourceToConsoleWithEventReportIT` never goes through that overload? A
one-line comment in the override noting it depends on being applied before
`start()` would also help prevent a future regression if someone switches the
test to the other factory.
The other items from the earlier review (F2/F3 close() robustness, F5
`takeRequest()` timeout / buffer leak / scheduler-interval assumption in the
test, F7 redirect following with report headers, F8 `; charset=utf-8` being
appended to the Content-Type) are still open as far as this thread shows —
please address them or reply per item so I can track them.
<!-- streview-comment:1164 -->
--
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]
