DanielLeens commented on PR #11809: URL: https://github.com/apache/seatunnel/pull/11809#issuecomment-5597688655
Addressed the remaining review items in `ab1538a9c1241741940cc789e88fb79bf18df608`: - F3: failed construction rolls back already-acquired listener registrations and rethrows the original failure. Tests cover failure at each of the three registrations and a removal failure during rollback. - F4: test-owned services now use try-with-resources, including the positive control and repeated create/close cases. - F5-F8: expected stopped-instance exceptions use fine logging, unexpected removal failures remain warnings with registration IDs; `AutoCloseable`, `@VisibleForTesting`, and the test-base type parameter are added. - F2: I retained the history-service reference and made the fresh-instance invariant explicit at both the getter and initialization site. This corrects my earlier proposed nulling: existing callers use the getter/read methods without a null fallback, and closing only deregisters listeners. Nulling the field would unnecessarily change that contract. A regression test verifies history reads still work after repeated close. Every activation constructs a new instance. The independent maintainer source review and module-scoped Spotless checks passed. GitHub confirms the new head and the four-file scope; there is no merge conflict. The new head's CI is queued, so this is not a claim of green CI. I also examined the previous head's failed jobs rather than treating all red jobs as flakes: - RocketMQ JDK 8 failed across engine variants with missing consume offsets/name-server routes, matching the shared test-container problem already addressed by open #12115. The JDK 11 RocketMQ job also failed; I have not independently attributed every JDK 11 failure from its log. - Engine JDK 8 failed in `SinkErrorToMysqlIT.testSinkMaxErrorRatioThreshold` because the post-job thread check still saw `HikariPool-3 housekeeper` and `st-multi-table-sink-writer-1` after two minutes. This is a resource-cleanup signal, not evidence that increasing the timeout or weakening the thread assertion is appropriate. The old failed jobs are superseded by this push; I did not queue redundant retries on the old SHA. The new run must establish the current CI result. No unrelated RocketMQ or sink-error production changes are included in this listener-lifecycle PR. -- 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]
