kroushan-nit commented on code in PR #952:
URL: https://github.com/apache/incubator-xtable/pull/952#discussion_r4183591165
##########
xtable-utilities/pom.xml:
##########
@@ -633,10 +640,10 @@
<include>org.apache.hudi:hudi-java-client</include>
<include>org.apache.hudi:hudi-timeline-service</include>
<include>org.apache.iceberg:iceberg-api</include>
+ <include>org.apache.iceberg:iceberg-aws</include>
Review Comment:
Second time this allowlist has silently shrunk the bundle — #841 was JOL and
SLF4J from the same #822 change, this is `iceberg-aws`.
`release/scripts/validate_bundled_license_texts.py` already reads the
bundled jar, the shade `<includes>` and the dependency tree, but it only walks
tree → includes (it iterates `tree_coords(tree)` and skips anything not in
`includes`). An `<include>` whose artifact has left the resolved graph is
invisible to it, which is exactly how `iceberg-hive-runtime` sat here doing
nothing.
Could it also assert the reverse — every `<include>` matches something in
the tree? That catches the whole class rather than this instance. It would need
the License Check job to cover `xtable-utilities`, which it currently skips.
##########
README.md:
##########
@@ -116,6 +116,7 @@ catalogOptions: # all other options are passed through in a
map
key1: value1
key2: value2
```
+The bundled jar includes Iceberg's `org.apache.iceberg.aws.glue.GlueCatalog`
and `org.apache.iceberg.aws.s3.S3FileIO`. Other catalog implementations must be
added to the classpath, which means running with `java -cp
xtable-utilities_2.12-0.5.0-SNAPSHOT-bundled.jar:my-catalog.jar
org.apache.xtable.utilities.RunSync ...` instead of `java -jar`.
Review Comment:
This reads as part of step 4 (catalog config), but it's about how to run,
and the `java -cp` alternative only makes sense against the `java -jar` line
right below it. Would it fit better after step 5, next to the existing "The
bundled jar includes hadoop dependencies…" sentence?
##########
xtable-utilities/pom.xml:
##########
@@ -200,6 +200,12 @@
<groupId>org.apache.iceberg</groupId>
<artifactId>iceberg-parquet</artifactId>
</dependency>
+ <!-- GlueCatalog and S3FileIO for icebergCatalogConfig. The AWS SDK
clients it needs are
Review Comment:
"already bundled below" holds for `s3` and `sts`, but `glue` isn't declared
in this pom — it arrives transitively from `xtable-aws`. Worth saying, since if
`xtable-aws` ever narrows that dependency the `software.amazon.awssdk:glue`
include goes stale exactly the way `iceberg-hive-runtime` just did, and nothing
here would fail.
--
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]
