rangareddy commented on code in PR #19433:
URL: https://github.com/apache/hudi/pull/19433#discussion_r3690169204
##########
packaging/hudi-hadoop-mr-bundle/pom.xml:
##########
@@ -136,7 +136,7 @@
<shadedPattern>org.apache.hudi.com.fasterxml.jackson.</shadedPattern>
</relocation>
</relocations>
- <createDependencyReducedPom>false</createDependencyReducedPom>
+ <createDependencyReducedPom>true</createDependencyReducedPom>
Review Comment:
Reproduced, and you are right on every part of this including why my
verification passed. Fixed in ed3ccf5.
Following your recipe exactly — install so the new POM lands in the
repository, then build presto **without** `-am`:
```
installed hudi-hadoop-mr-bundle POM: 0 <dependency> entries
unzip -l hudi-presto-bundle-1.3.0-SNAPSHOT.jar | grep -c
org/apache/hudi/hadoop/
before this PR : 109 (HoodieParquetInputFormat.class present)
with reduced POM: 0 (HoodieParquetInputFormat.class gone)
```
And you correctly diagnosed my earlier "verification": `~/.m2` still held
the un-reduced POM with its 5 entries, so `mvn package -pl
packaging/hudi-aws-bundle` never exercised the reduced metadata at all. Noted
for the future — for this kind of change the only meaningful check is
install-then-resolve-from-repository.
Two changes:
- `hudi-presto-bundle` now declares `hudi-hadoop-mr` and
`hudi-hadoop-common` directly. Re-ran the same recipe: back to **109** entries
with `HoodieParquetInputFormat.class` present, built without `-am` against the
reduced POM. Taking your point that this stands on its own merit regardless of
this PR.
- `promoteTransitiveDependencies=true` on all seven bundles, for your point
2. The `hudi-hadoop-mr-bundle` reduced POM goes from **0 to 79** dependencies,
restoring exactly what you listed — `orc-core`, `rocksdbjni`,
`jetty-server`/`jetty-servlet`, dropwizard metrics, `simpleclient`,
`tally-core`, `lz4-java`, `disruptor`, `jaxb-api`.
The CI observation is worth keeping on the record too: every `-pl
packaging/...` in `.github/workflows` and `scripts/` passes `-am`, and
`packaging/bundle-validation` has no presto coverage, so nothing here would
have caught it. Happy to file that as a follow-up if you think a presto entry
in bundle-validation is wanted.
--
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]
