danielcweeks commented on code in PR #18660:
URL: https://github.com/apache/iceberg/pull/18660#discussion_r4541394418
##########
kafka-connect/build.gradle:
##########
@@ -253,6 +257,11 @@
project(':iceberg-kafka-connect:iceberg-kafka-connect-runtime') {
it.file in [hiveDistZip.archiveFile.get().asFile,
hiveDistTar.archiveFile.get().asFile]
}
+ // track main and hive distributions separately; set properties before each
apply
+ ext.runtimeDepsFile = 'main/runtime-deps.txt'
+ apply from: "${rootDir}/runtime-deps.gradle"
+ ext.runtimeDepsFile = 'hive/runtime-deps.txt'
+ ext.runtimeDepsConfiguration = configurations.hive
apply from: "${rootDir}/runtime-deps.gradle"
Review Comment:
I'd rather not pass parameters by mutating `ext`. Could we add a function
like `trackRuntimeDeps(String file, Configuration conf)` that we call twice
here, with other modules keeping the single default call?
<!-- lenses: weeks-review, russell-review, blue-review -->
##########
kafka-connect/kafka-connect-runtime/hive/runtime-deps.txt:
##########
@@ -0,0 +1,352 @@
+aopalliance:aopalliance:1.0
+ch.qos.logback:logback-classic:1.2
+ch.qos.logback:logback-core:1.2
Review Comment:
This was already the case before: hive ships
`logback-classic`/`logback-core` and `hive/LICENSE` has no Logback entry. Both
logback 1.2 and `slf4j-reload4j` 1.7 are SLF4J 1.x bindings, so they won't bind
against slf4j-api 2.0 anyway.
Could we exclude them from `hive`, or at least add the LICENSE entry?
##########
kafka-connect/build.gradle:
##########
@@ -74,6 +74,10 @@
project(':iceberg-kafka-connect:iceberg-kafka-connect-runtime') {
hive {
extendsFrom runtimeClasspath
}
+ runtimeClasspath {
+ // keep the hive distribution a superset of main by resolving shared
jars to the same versions
+ shouldResolveConsistentlyWith(hive)
+ }
Review Comment:
I don't think we need this since `hive` already `extendsFrom
runtimeClasspath`, so it's a superset.
<!-- lenses: weeks-review, russell-review, blue-review -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
