DanielLeens commented on PR #11902:
URL: https://github.com/apache/seatunnel/pull/11902#issuecomment-5379751856
@goutamadwant Confirmed — you're right, and I checked this directly rather
than taking it on faith.
**The claim checks out.** Current `dev`'s root `pom.xml` no longer has a
`seatunnel-shade` reactor module at all (`git ls-tree dev -- seatunnel-shade`
is empty), and `docs/en/developer/shade-guide.md` (added by #9993, "Refactor
the seatunnel-shade module") documents why: all `seatunnel-shade-*` artifacts,
including `seatunnel-shade-hadoop3-uber`, are now built in a separate
`apache/seatunnel-shade` repo and consumed as plain Maven Central dependencies
(`org.apache.seatunnel:seatunnel-shade-hadoop3-uber:3.1.4-3.0.0` in current
`dev`'s dependencyManagement). This PR's own
`seatunnel-shade/seatunnel-hadoop3-3.1.4-uber` module path simply doesn't exist
on `dev` anymore — so this is correctly diagnosed as more than a rebase.
**I also checked the actual target module in `apache/seatunnel-shade`.** Its
current `seatunnel-shade-hadoop3-uber/pom.xml` does not include `hadoop-azure`
at all yet, so ABFS support does need to land there first, in that repo, before
it can reach this PR.
**Your "clean route" is the right one, and I can show why from that module's
own config**: `seatunnel-shade-hadoop3-uber` already relocates
`com.google.common` and `com.fasterxml.jackson` for the whole shaded jar (not
per source-artifact), so adding `hadoop-azure` as a dependency *inside that
module* — exactly the shape of the change you already wrote for the old in-repo
location — keeps its Guava/Jackson usage relocated too. Declaring
`hadoop-azure` directly in the connector module instead would bypass that
relocation entirely, which is exactly the classpath pollution you flagged.
One thing worth double-checking when you port the change over: that module's
Guava relocation is scoped with `<includes>` to only
`com.google.common.base.*`, `.cache.*`, `.collect.*` — not all of Guava. If
`hadoop-azure` or its Azure SDK dependencies touch other Guava packages
(`util.concurrent`, `io`, `net`, etc.), those would stay unrelocated even
inside the shade jar, so it's worth verifying which Guava packages
`hadoop-azure` actually pulls in before assuming the existing include list is
sufficient.
Also — please carry over the fix for the issue I raised in my original
review, don't just re-paste the old diff: the current `apache/seatunnel-shade`
copy of this module's shade-plugin config has **no `<transformers>` block at
all**, same as before. If you add the `hadoop-azure` include filter plus
hand-added
`META-INF/services/org.apache.hadoop.security.token.{TokenIdentifier,TokenRenewer}`
files the same way this PR originally did, you'll reproduce the exact
collision I flagged — HDFS's own `DelegationTokenIdentifier`/`TokenRenewer`
registrations from `hadoop-hdfs-client` silently getting dropped in favor of
the ABFS-only ones. The correct fix there is a `ServicesResourceTransformer` in
that execution's `<configuration>` so both sets of service entries get merged,
not hand-added files that collide with the transitive jar's own entries.
So, concretely: yes, treat this as a two-repository change.
1. Open a PR against `apache/seatunnel-shade` that ports this PR's
dependency + filter change into `seatunnel-shade-hadoop3-uber`, but fixes the
META-INF/services handling with a proper `ServicesResourceTransformer` instead
of hand-added files.
2. Since this changes the shade-plugin configuration (not just a library
version), per that repo's own release guidance you'll need a version bump
before it can be deployed — the current `3.1.4-3.0.0` coordinate is already
published, and redeploying to the same coordinate will be rejected.
3. Once released, Maven Central needs roughly a day to propagate before
`apache/seatunnel`'s CI can resolve the new artifact.
4. Then rebase this PR onto current `dev`, drop the now-defunct in-repo
`seatunnel-shade/seatunnel-hadoop3-3.1.4-uber` diff entirely (it's dead weight
against current `dev`), and just bump the corresponding version property in the
root `pom.xml` to pick up the new published artifact.
--
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]
