You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2023/01/12 09:02:02 UTC

[GitHub] [spark] grundprinzip opened a new pull request, #39531: [SPARK-XXXX][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

grundprinzip opened a new pull request, #39531:
URL: https://github.com/apache/spark/pull/39531

   ### What changes were proposed in this pull request?
   When building a release package for PySpark and manually adding the `spark-connect_2.12-3.4.0-SNAPSHOT.jar` jar to the distribution. The PySpark shell will not start when started using the following option:
   
   ```
   > psypark --remote local
   py4j.protocol.Py4JJavaError: An error occurred while calling None.org.apache.spark.api.java.JavaSparkContext.
   : java.lang.NoClassDefFoundError: org/sparkproject/guava/util/concurrent/internal/InternalFutureFailureAccess
   	at java.lang.ClassLoader.defineClass1(Native Method)
   	at java.lang.ClassLoader.defineClass(ClassLoader.java:757)
   	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
   	at java.net.URLClassLoader.defineClass(URLClassLoader.java:473)
   	at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
   	at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
   	at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
   	at java.security.AccessController.doPrivileged(Native Method)
   	at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
   	at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
   ```
   
   The reason is that the `connect-common` package applies the default shading rules of Spark and thus we dont pickup the right dependency for the connect package. This patch fixes this issue by applying the same shading rules for `guava` to `connect-common`.
   
   ### Why are the changes needed?
   Release distribution
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Manual testing
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39531:
URL: https://github.com/apache/spark/pull/39531#issuecomment-1380115732

   cc @LuciferYang too if you find some time to take a look for a posthoc 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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon closed pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure
URL: https://github.com/apache/spark/pull/39531


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1068958012


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   > I will test this guess manually
   
   After manual check, configuring
   ```
   <artifactSet>
     <includes>
       <include>com.google.guava:*</include>
     </includes>
   </artifactSet>
   ```
   is redundant
   
   



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39531:
URL: https://github.com/apache/spark/pull/39531#issuecomment-1380115158

   Merged to master.
   
   cc @hvanhovell FYI


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1068000433


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   I will test this guess manually
   
   



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1067997804


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   If `connect-common.jar` will not be used independently, I guess it can be used only by configuring the `relocations` rule(same as `connect` module), the `artifactSet` doesn't need to be configured.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1068010004


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   Thanks for reviewing this thoroughly, @LuciferYang 



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1068958012


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   > I will test this guess manually
   
   After manual check server side,  configuring
   ```
   <artifactSet>
     <includes>
       <include>com.google.guava:*</include>
     </includes>
   </artifactSet>
   ```
   is redundant
   
   



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39531: [SPARK-42029][CONNECT] Add Guava Shading rules to `connect-common` to avoid startup failure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39531:
URL: https://github.com/apache/spark/pull/39531#discussion_r1069067241


##########
connector/connect/common/pom.xml:
##########
@@ -156,6 +156,43 @@
                     </execution>
                 </executions>
             </plugin>
+            <!-- Shade all GRPC / Guava / Protobuf dependencies of this build -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <configuration>
+                    <shadedArtifactAttached>false</shadedArtifactAttached>
+                    <artifactSet>

Review Comment:
   Thank you. Would you like to create a followup if you don't mind?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org