SEZ9 commented on PR #11077:
URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-6207433039
Hi @hesam-oxe, thanks for the follow-up and for offering to add a
multi-table test case.
On "flag the failing check": there is nothing to flag yet, because CI never
ran on `8b383c04eeed65c41a14ba756100a33f430124eb`. The apache-side `Build`
check for that head is `action_required` ("Workflow run detection failed"), and
there is no workflow run for the commit in your fork either (reference:
https://github.com/apache/seatunnel/runs/114987916217). To get it going:
1. Enable GitHub Actions workflows in your fork (Actions tab, enable button
if shown).
2. Sync the latest `dev` into your branch (it is currently `diverged`, ahead
38 / behind 24). When resolving conflicts, please keep every existing entry in
both `incompatible-changes.md` files.
3. Push the result (an empty commit is fine if nothing else changes) or
start the `Build` workflow manually on the branch.
On the review items: I still see the same head I reviewed, so the earlier
findings remain open. In the same push, it would be great if you could cover:
- Destination-key collisions and `proxyContexts`: a test that pins the
identifier components now used for the destination key (hdfsNameKey, tmp_path,
file_name_expression, one extraOptions entry) and shows that two tables with
different physical destinations never share a writer, plus the
metadata-only-schema sharing case. This also answers the collision-test
question raised earlier.
- Snapshot fan-out and restore union: a legacy-restore test showing a shared
writer's state is restored exactly once, not N times, when multiple aliased
identifiers are present in the checkpoint.
- Writer created from an arbitrary alias: a short note on how schema/config
divergence across aliased tables is guarded, or a test showing the behavior.
- Docs: document `getPhysicalDestinationIdentifier()` and the changed
`restoreWriter` contract (merged state from all aliased identifiers passed in
one call), and please make the `incompatible-changes.md` entries (en and zh)
and the release note describe the real behavior. As written they promise a
startup `IllegalStateException` that cannot occur for the file sink, since the
row layout is already folded into its destination identifier.
- The IOException wrapped in RuntimeException inside `computeIfAbsent`, and
the missing param/return Javadoc tags on `getDestinationKey`: small fixes, but
still outstanding on this head.
If any of these are already addressed in a commit I'm not seeing, just point
me at it. Once the new head is up and CI has started, I'll re-check the full
diff.
<!-- streview-comment:1795 -->
--
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]
