DanielLeens commented on PR #11077:
URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5696102378
@SEZ9 @hesam-oxe Thanks both — a few notes to close this out, verified
against the actual diffs rather than re-stated.
**On the "cut off" comment**: my 09-15 comment (`5674345945`) wasn't
actually truncated on my end — it ends with "...the F1-F8 set closed on
`a9d8464` and stayed closed through the sync." followed by the "no open
blockers" paragraph. Might have been a rendering hiccup; worth a page reload if
it still looks cut off for you.
**On F5/F7 provenance** (@SEZ9's question) — traced it precisely instead of
repeating "not reproducible": both were fixed in `265e16cddd9` ("[Fix][API]
Complete shared multi-table sink lifecycle", 2026-08-25), not `a9d8464`:
- F5: the old `if (!proxyContexts.containsValue(proxy))` gate (and its O(n²)
scan) was removed there, replaced by the unconditional `proxyContexts.put(id,
proxy)` per alias that's still on head.
- F7: `sink.createWriter(proxy)` moved out of
`destinationWriters.computeIfAbsent(...)` into a `writer == null` check inside
a `try { ... } catch (IOException error)` block, so the checked exception
propagates instead of being wrapped in `RuntimeException`.
So: never present on the head you originally reviewed, fixed ~3 weeks before
`a9d8464`, and I re-confirmed both are still fixed on `ef2bb0955f3` by reading
the current source directly.
**Docs (F4/F6)**: confirmed in the diff itself. `git diff
bd09f6d9ae9...ef2bb0955f3 --stat` shows
`docs/en/developer/sink-connector-development.md` (+17) and
`docs/zh/developer/sink-connector-development.md` (+14) as two of this PR's own
15 changed files — not a follow-up.
**On the CI evidence** (@hesam-oxe) — pulled both runs myself (fork
`34826109614` vs. `dev` baseline `34935118103`) rather than taking the summary
at face value, since CI was the last open gate on my side. Most of it holds up:
- `engine-v2-it`:
`CheckpointCoordinatorFailoverIT.testStreamJobFailsAfterCheckpointTriggerDispatchFailure`
and
`BackpressureSlowSinkIT.testCheckpointsKeepCompletingUnderSustainedBackpressure`
both fail on the `dev` baseline too, just landing on different shard numbers
(8/11 swap between runs) — not new, matches the flakes already tracked in this
thread.
- `Dead links`: confirmed transient `503` from the link checker, not a real
dead link.
One correction on `all-connectors-it-7`: it is not literally the same
failure as the tracked flake, so I want to be precise rather than let the
summary stand as "identical." On this head, both shards fail on
`S3FileConnectDryRunIT.startUp:80` with `ContainerLaunchException` → `pull
access denied for minio/minio`. On the `dev` baseline, `all-connectors-it-7(8)`
fails on an unrelated `AmazondynamodbIT` assertion, and `(11)` actually passes.
Root cause: `S3FileConnectDryRunIT` (added by #12242, merged into `dev` after
the #12287 MinIO-mirror fix) hardcodes
`minio/minio:RELEASE.2024-06-13T22-53-53Z` directly at line 59 instead of going
through whatever #12287 patched, so it independently regressed the same Docker
Hub 404. It's real and it's infra — nothing to do with this PR's
`connector-file-base` changes (that test file isn't in this diff, and the
failure happens in `startUp()`, a container-launch step, before any sink code
runs) — but "identical to the tracked flake" wasn't q
uite accurate, so flagging the distinction for the record. I'll raise that
hardcoded image separately; no action needed here for it.
**Net**: from my side this PR's own code still has no open blockers — F1-F8
all confirmed resolved and now re-verified against the actual commit diffs
rather than just re-asserted — and the outstanding CI failures are
pre-existing/environmental, matching `dev` baseline behavior with the one
correction above. I don't have merge authority here; next step is a maintainer
to merge once CI is accepted as environmentally red the same way `dev` itself
currently is, or after an off-peak rerun.
--
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]
