FrankChen021 commented on code in PR #20409:
URL: https://github.com/apache/druid/pull/20409#discussion_r4100441195


##########
cloud/aws-common/pom.xml:
##########
@@ -100,6 +100,22 @@
             <version>${aws.sdk.v2.version}</version>
             <scope>runtime</scope>
         </dependency>
+        <!--
+          Promoted to the shared classpath (rather than left as an 
extension-only dependency) so that only
+          one copy of the AWS SDK S3 client classes is ever loaded by the JVM. 
software.amazon.awssdk:s3
+          registers static IdentityProperty constants (e.g. in 
S3ExpressPlugin) into a shared, SDK-wide
+          registry. When druid-s3-extensions and druid-iceberg-extensions are 
loaded together, each bundles
+          its own copy of this jar and each gets its own extension 
classloader, so the registry ends up with
+          two IdentityProperty instances that have the same name/namespace but 
different object identity,
+          and throws IllegalArgumentException: "No duplicate IdentityProperty 
names allowed". See
+          https://github.com/apache/druid/issues/19636.
+        -->
+        <dependency>

Review Comment:
   The change is correct but I think the comment is too verbose, a concise one 
can be:
   
   > Keep S3 classes on the shared classpath to avoid loading separate copies 
in the different extensions.
   
   please update other comments accordingly.



-- 
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]

Reply via email to