rangareddy commented on code in PR #19433:
URL: https://github.com/apache/hudi/pull/19433#discussion_r3690172522
##########
packaging/hudi-aws-bundle/pom.xml:
##########
@@ -117,7 +117,7 @@
<shadedPattern>org.apache.hudi.aws.org.apache.httpcomponents.</shadedPattern>
</relocation>
</relocations>
-
<createDependencyReducedPom>false</createDependencyReducedPom>
+
<createDependencyReducedPom>true</createDependencyReducedPom>
Review Comment:
Agreed on the mechanism, and my Impact wording was wrong — "artifacts it
actually bundled" is not what `promoteTransitiveDependencies=false` does. Fixed
in ed3ccf5 by taking your first option, `promoteTransitiveDependencies=true`,
on all seven bundles, so the reduced POM keeps the runtime contract minus what
was absorbed. Effect is visible on `hudi-hadoop-mr-bundle`, whose reduced POM
goes from 0 to 79 dependencies (orc-core, rocksdbjni, jetty, metrics,
simpleclient, lz4-java, disruptor, jaxb-api, …). The other bundles land at
74–191.
One correction on the specific example, though, because it changed what I
expected to see. Those seven artifacts are **not** in `hudi-aws-bundle`'s
dependency closure, before or after this PR. They are `provided` in the root
pom's `dependencyManagement`:
```
hadoop-client, hadoop-common, hadoop-hdfs, hadoop-auth,
hive-jdbc, hive-metastore, hive-common -> <scope>provided</scope>
```
`provided` is not transitive, so they never propagate from `hudi-hive-sync`
to this bundle. `mvn dependency:list -pl packaging/hudi-aws-bundle` returns
exactly one `org.apache.hive`/`org.apache.hadoop` artifact in the whole tree —
`hive-storage-api:2.6.0:compile` — and no `hadoop-*` at all. So removing the
`hudi-hive-sync` entry from the published POM was not dropping them; they were
never advertised to consumers in the first place.
They are compile scope where you looked, in
`hudi-sync/hudi-hive-sync/pom.xml`, which is what I assume produced the list.
The root-level management is what makes them non-transitive one level up.
That does not change the outcome here: your general point was correct,
non-bundled transitives were being dropped, and
`promoteTransitiveDependencies=true` now preserves them. It only means the
classes in this bundle that call into Hadoop/Hive were relying on a `provided`
classpath before this PR as well, which is the normal contract for a bundle
meant to land on a Hadoop/Hive cluster. Say the word if you would rather I also
called that out in the Impact section — I have rewritten it, but I did not want
to imply this PR changed that part.
And agreed on `hudi-common` and `parquet-avro` being correctly kept.
--
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]
