DanielLeens commented on PR #11626:
URL: https://github.com/apache/seatunnel/pull/11626#issuecomment-5473725512
+1, agreed, @SEZ9 — your Issues 1/2 here (F5/F6) and my Issue 1 from the
2026-08-29 review of this same head (`9009761ba8b1`) are the same underlying
gap: `assertNoRunningJobs()` still doesn't actually retry.
One added technical detail on top of what you flagged, since it explains
*why* the commit's stated fix ("REST checks are bounded") looks complete but
isn't: the new wrapper at `SeaTunnelContainer.java:399-404` is
`Awaitility.await().atMost(30, TimeUnit.SECONDS).pollInterval(1,
TimeUnit.SECONDS).untilAsserted(this::assertNoRunningJobsOnce)`, but
`assertNoRunningJobsOnce()` still signals failure via a plain `throw new
IllegalStateException(...)` (`:424-429`, `:430-433`) rather than an
`AssertionError`. I verified against the resolved `awaitility-4.2.2` jar this
module pulls in that `untilAsserted` only treats `AssertionError` as "not yet
satisfied, keep polling" — any other `Throwable`, `IllegalStateException`
included, propagates on the very first attempt with zero elapsed poll time. So
the transient-lag race you're both describing isn't just unmitigated, it's
mitigated in appearance only: the Awaitility wrapper reads as a fix but never
gets a chance to retry.
The existing codebase pattern two call sites down in the same file
(`:607-625`, `:704-716`) already does this correctly with
`Assertions.assertTrue(...)`, which throws `AssertionError` — switching
`assertNoRunningJobsOnce()` to the same pattern (or catching
`IllegalStateException` inside the `untilAsserted` lambda and rethrowing as
`AssertionError`) closes this with a one-line change.
Merge conclusion unchanged from my 2026-08-29 review: Ready to merge after
fixes, with this as the sole remaining blocker (High severity) on the current
head.
--
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]
