SEZ9 commented on PR #11673: URL: https://github.com/apache/seatunnel/pull/11673#issuecomment-5225134266
Thanks @loustler — I've reviewed `fb7697e2f` and both of my findings are resolved. **Issue 1:** Agreed with your reasoning for Option A. Your framing is the right one: the static check asserts exactly one invariant, and everything else a bad relocation can break only surfaces at runtime — the #11648 `okio`/k8s-e2e incident is a convincing precedent. The ~1% commit frequency makes the full-matrix cost acceptable for this blast radius. Good catch on the `hadoop3.version` triple-declaration too; you're right that `seatunnel-core/**` coverage there is incidental rather than guaranteed, which validates the explicit `seatunnel-shade/**` entry. **Issue 2:** The narrowed mask with the named `SHADED_PREFIX` constant pointing back at the pom property is exactly what I was after, and re-running the four controls after the mask change was the right instinct — the patched-jar PASS plus the 3.4.3 FAIL confirms the check still has teeth. Ordering fix confirmed on my end as well. **Remaining items — all non-blocking, per my alignment with @SEZ9's review:** 1. Extend the guard (or add a sibling check) to also verify the uber jar still relocates Jackson. As noted, a future uber-jar shading change could reintroduce the same `NoSuchMethodError` from the other direction and the current step wouldn't catch it. Fine as a follow-up PR if you'd prefer to keep this one scoped. 2. A small unit test for the masking regex, so future edits to `SHADED_PREFIX`/`RELOCATED` can't quietly weaken the check. 3. The minor doc/style nits from my earlier review (`python3` vs `python`, and moving the lockstep convention out of comments into docs). The core fix and the guard as they stand correctly close #11655. Once CI is green on the final head with the expanded matrix this PR now triggers for itself, this is good to merge from my side. <!-- streview-comment:87 --> -- 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]
