DanielLeens commented on PR #11103: URL: https://github.com/apache/seatunnel/pull/11103#issuecomment-5658356185
Status-update check-in on the new commit since my last review. **What changed**: the only new commit (`8b599b94a0`) is a mechanical `Merge remote-tracking branch 'upstream/dev' into pr-11103-round-20260909T064221Z` — no independent logic of its own. I diffed the pre-merge tip (`93708890a8`, the head from my last "Ready to merge (pending CI)" review) against this merge commit directly (`git diff 93708890a8 8b599b94a0`) rather than trusting the commit title: - The only file this PR actually owns that the merge touched is `AbstractWriteStrategy.java`. The delta there is entirely `dev`'s new `RestoreTableSchemaEvent` handling (an unrelated schema-restore feature merged from `dev`), applied as a clean, additive auto-merge — new import, a new branch in `applySchemaChange()`, and a new `restoreSinkColumnNames()` helper. It does not touch, revert, or interact with anything from this PR's fix. - I re-verified every `synchronized` this PR added is still present on the merge-commit head: `OrcWriteStrategy`/`ParquetWriteStrategy`/`TextWriteStrategy`/`BinaryWriteStrategy` `write()`/`finishAndCloseFile()`, `BinaryWriteStrategy.applySchemaChange()`, and `AbstractWriteStrategy.prepareCommit()`/`abortPrepare()`/`abortPrepare(String)`/`snapshotState()`. Nothing was silently dropped by the merge. - No source-level blockers remain open from my side (the last real one — `Orc`/`Parquet.write()` missing `synchronized` — was closed on `93708890a8`, and I re-confirmed that fix is still in place above). **CI status**: `Build` is currently **failing** on this head (`8b599b94a0`), so I want to be precise rather than just report "still red" — I checked the actual failing jobs on the fork run and both are unrelated to this PR's file-sink changes: - `unit-test (11, windows-latest)` / `unit-test (8, ubuntu-latest)`: fails in `org.apache.seatunnel.edge.agent.connector.file.FileCollectReaderBehaviorTest.rediscoversFileAfterInactiveCursorClosed` — a timing-based Awaitility assertion in the unrelated `seatunnel-edge-agent` module, not touched by this diff. - `updated-modules-integration-test-part-5`: fails in `S3FileConnectDryRunIT`/`S3FileConnectDryRunWireIT` (also untouched by this PR) with `pull access denied for minio/minio` — this is the same Docker Hub `minio/minio` image removal that `#12287` fixed for most call sites; this newly-added S3 dry-run test (merged into `dev` after `#12287`) appears to be one of the sites still pointing at the old `minio/minio` tag rather than the `quay.io` mirror. Both failure classes are dev-wide/environmental, not caused by anything in this PR, and I wouldn't want a business-code change made here to "fix" either of them. From a source-review perspective this remains **ready to merge**; the path forward is a job-level rerun (or picking up `dev` once the `S3FileConnectDryRunIT` image reference is fixed) rather than any change to this branch. As before, I only have comment-only access here, so a write-capable maintainer still needs to give the binding approval and merge once CI is green. -- 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]
