voonhous commented on code in PR #19425:
URL: https://github.com/apache/hudi/pull/19425#discussion_r3803137209
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -153,6 +153,43 @@
<include>com.101tec:zkclient</include>
<include>org.apache.kafka:kafka-clients</include>
+ <!-- AWS SDK v2 core, shared by the kinesis and sts clients
below; relocated further down. -->
+ <include>software.amazon.awssdk:annotations</include>
+ <include>software.amazon.awssdk:utils</include>
+ <include>software.amazon.awssdk:sdk-core</include>
+ <include>software.amazon.awssdk:aws-core</include>
+ <include>software.amazon.awssdk:auth</include>
+ <include>software.amazon.awssdk:http-client-spi</include>
+ <include>software.amazon.awssdk:regions</include>
+ <include>software.amazon.awssdk:metrics-spi</include>
+ <include>software.amazon.awssdk:json-utils</include>
+ <include>software.amazon.awssdk:endpoints-spi</include>
+ <include>software.amazon.awssdk:retries</include>
+ <include>software.amazon.awssdk:retries-spi</include>
+ <include>software.amazon.awssdk:checksums</include>
+ <include>software.amazon.awssdk:checksums-spi</include>
+ <include>software.amazon.awssdk:identity-spi</include>
+ <include>software.amazon.awssdk:http-auth</include>
+ <include>software.amazon.awssdk:http-auth-spi</include>
+ <include>software.amazon.awssdk:http-auth-aws</include>
+
<include>software.amazon.awssdk:http-auth-aws-eventstream</include>
+ <include>software.amazon.awssdk:protocol-core</include>
+ <include>software.amazon.awssdk:apache-client</include>
+ <include>software.amazon.awssdk:netty-nio-client</include>
Review Comment:
`netty-nio-client` is dead weight here: 169 relocated classes whose
`io.netty.*` references are not bundled (about 800 dangling refs), and the sync
`KinesisClient` never touches it (`-verbose:class` on a live `listShards` shows
0 netty loads, 46 apache-client loads). The PR body's "none of that is
optional" does not hold for it. `http-auth-aws-eventstream` (line 175) has the
same shape: it references `software.amazon.eventstream.*`, which is not
included, and `http-auth-aws` itself carries 12 event-stream signer classes
with the same reference.
Please drop `netty-nio-client` and `http-auth-aws-eventstream`, and either
add `software.amazon.eventstream:eventstream` (tiny; relocate it too so nothing
dangles) or state the accepted latent gap in a comment. Keep
`org.reactivestreams:reactive-streams`: `Publisher`/`Subscriber` are loaded on
the sync path.
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -235,6 +272,10 @@
<pattern>org.apache.httpcomponents.</pattern>
<shadedPattern>org.apache.hudi.aws.org.apache.httpcomponents.</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>software.amazon.awssdk.</pattern>
Review Comment:
This relocation rewrites every class in the jar, including `S3EventsSource`
/ `CloudObjectsSelector` / `S3EventsMetaSelector`, whose 13
`software.amazon.awssdk.services.sqs.*` imports become
`org.apache.hudi.software.amazon.awssdk.services.sqs.*` -- and `sqs` is not in
the include list (the PR body lists its absence as a success criterion). On
master those references are unrelocated and satisfied by a runtime-provided SDK
v2 (hadoop-aws 3.4+/EMR, or the `aws-java-sdk-sqs` jar the S3-events docs tell
users to download); after this PR they resolve only if `hudi-aws-bundle`
happens to be on the classpath too. Verified on the built jar: `javap -c` on
the bundled `CloudObjectsSelector` shows 11 references to the relocated
`SqsClient`, and `unzip -l | grep services/sqs` is 0.
Please add `<include>software.amazon.awssdk:sqs</include>` (its closure --
`aws-json-protocol`, `protocol-core`, `http-auth*`, `identity-spi` -- is
already listed) so every SDK reference the bundle's own classes emit resolves
in-jar, and drop "sqs absent" from the verification list in the PR body.
--
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]
