You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Yikf (via GitHub)" <gi...@apache.org> on 2023/10/18 15:11:52 UTC

[PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error "java.lang.ClassNotFoundException: org.sparkproject.guava.util.concurrent.internal.InternalFutureFailureAccess" [spark]

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

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'core/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   
   Fix a build issue, when building a runnable distribution from master code running spark-sql raise error:
   ```
   Caused by: java.lang.ClassNotFoundException: org.sparkproject.guava.util.concurrent.internal.InternalFutureFailureAccess
   	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
   	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
   	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
   	... 58 more
   ```
   the problem is due to a gauva dependency in  spark-connect-common POM that **conflicts**  with the shade plugin of the parent pom.
   
   - the spark-connect-common contains `connect.guava.version` version of guava, and it is relocation as `${spark.shade.packageName}.guava` not the `${spark.shade.packageName}.connect.guava`;
   - The spark-network-common also contains guava related classes, it has also been relocation is `${spark.shade.packageName}.guava`, but guava version `${guava.version}`;
   - As a result, in the presence of different versions of the classpath org.sparkproject.guava.xx;
   
   In addition, after investigation, it seems that module spark-connect-common is not related to guava, so we can remove guava dependency from spark-connect-common.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Building a runnable distribution from master code is not runnable.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   No
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   I ran the build command output a runnable distribution package manually for the tests; 
   
   Build command:
   ```
   ./dev/make-distribution.sh --name ui --pip --tgz  -Phive -Phive-thriftserver -Pyarn -Pconnect
   ```
   
   Test result:
   <img width="1276" alt="image" src="https://github.com/apache/spark/assets/51110188/aefbc433-ea5c-4287-8ebd-367806043ac8">
   
   I also checked the `org.sparkproject.guava.cache.LocalCache` from jars dir;
   Before:
   ```
   ➜  jars grep -lr 'org.sparkproject.guava.cache.LocalCache' ./
   .//spark-connect_2.13-4.0.0-SNAPSHOT.jar
   .//spark-network-common_2.13-4.0.0-SNAPSHOT.jar
   .//spark-connect-common_2.13-4.0.0-SNAPSHOT.jar
   ```
   
   Now:
   ```
   ➜  jars grep -lr 'org.sparkproject.guava.cache.LocalCache' ./
   .//spark-network-common_2.13-4.0.0-SNAPSHOT.jar
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   No


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1447185917


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -137,6 +137,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   BTW, I also tested connect as described in [PR description](https://github.com/apache/spark/pull/43195#issue-1921234067) , and spark sql shell using `./bin/spark-sql` , and it worked as expected.



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "c3-ffomenko (via GitHub)" <gi...@apache.org>.
c3-ffomenko commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1446636964


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   Thank you for working on this PR, it helped us with our issue. We had additional problem with same error message when we build spark with upgraded version of guava which depends on failureaccess. In this case shading done in network common misses failureaccess dependency since shading is done only on guava omitting classes in failureaccess.



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372544571


##########
assembly/pom.xml:
##########
@@ -149,6 +149,12 @@
           <groupId>org.apache.spark</groupId>
           <artifactId>spark-connect_${scala.binary.version}</artifactId>
           <version>${project.version}</version>
+          <exclusions>
+            <exclusion>
+              <groupId>org.apache.spark</groupId>
+              <artifactId>spark-connect-common_${scala.binary.version}</artifactId>

Review Comment:
   Runnable distribution tar already contains connect-server and connect-server shade the connect-common, so runnable distribution tar does not need to contain connect-common anymore.
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/connector/connect/server/pom.xml#L306



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1446926077


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -137,6 +137,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   still need this change?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1884249245

   Any other suggestion on the PR? @hvanhovell @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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372477645


##########
assembly/pom.xml:
##########
@@ -149,6 +149,12 @@
           <groupId>org.apache.spark</groupId>
           <artifactId>spark-connect_${scala.binary.version}</artifactId>
           <version>${project.version}</version>
+          <exclusions>
+            <exclusion>
+              <groupId>org.apache.spark</groupId>
+              <artifactId>spark-connect-common_${scala.binary.version}</artifactId>

Review Comment:
   Why this change?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1364927001


##########
connector/connect/common/pom.xml:
##########
@@ -47,18 +47,6 @@
             <groupId>com.google.protobuf</groupId>
             <artifactId>protobuf-java</artifactId>
         </dependency>
-        <dependency>

Review Comment:
   Both connect-server and connect-client  have their own independent guava dependencies : )
   
   I think we should use connect-common shade guava, connect-client and connect-server shade connect-common to make connect-server and connect-client use the same guava dependency. 
   
   In addition, because connect-common is included in the connect-server shade, the runnable distribution can also exclude connect-common. 
   
   Do you like this current commit?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1447182488


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -137,6 +137,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   you are right, It is unnecessary. 
   
   Now, we perform shading of the Guava library within the connect-common module to ensure both connect-server and connect-client modules maintain consistent and accurate Guava dependencies.



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1364346394


##########
connector/connect/common/pom.xml:
##########
@@ -47,18 +47,6 @@
             <groupId>com.google.protobuf</groupId>
             <artifactId>protobuf-java</artifactId>
         </dependency>
-        <dependency>

Review Comment:
   Spark connect requires a higher version of guava, so we can't remove this dependency. We should reconfigure the maven-shade-plugin in connect-common to exclude the packaging of this Guava.
   
   



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1365428396


##########
connector/connect/common/pom.xml:
##########
@@ -47,18 +47,6 @@
             <groupId>com.google.protobuf</groupId>
             <artifactId>protobuf-java</artifactId>
         </dependency>
-        <dependency>

Review Comment:
   I tested it a s a's description and it worked as scheduled
   I tested as https://github.com/apache/spark/pull/43195#issue-1921234067, i 
   <img width="1440" alt="image" src="https://github.com/apache/spark/assets/51110188/0176bb24-264e-4e77-997e-d4cafa9b5aa6">
   



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1365428396


##########
connector/connect/common/pom.xml:
##########
@@ -47,18 +47,6 @@
             <groupId>com.google.protobuf</groupId>
             <artifactId>protobuf-java</artifactId>
         </dependency>
-        <dependency>

Review Comment:
   I tested it as https://github.com/apache/spark/pull/43195#issue-1921234067 description and it worked as expected.
   <img width="1440" alt="image" src="https://github.com/apache/spark/assets/51110188/0176bb24-264e-4e77-997e-d4cafa9b5aa6">
   



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372554800


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   The current Apache Spark includes two versions of Guava, which are (The historical reasons are not clear to me. @LuciferYang may know some background):
   1. https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/pom.xml#L203
   2. https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/pom.xml#L292
   
   The current Apache Spark has shading in the following three places for these two versions:
   1. for guava.version
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/common/network-common/pom.xml#L125-L129
   2. for connect.guava.version
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/connector/connect/server/pom.xml#L310-L316
   3. for conflicting versions, the connect-common does not override the parent's shade plugin, so it is relocated to '${spark.shade.packageName}.guava'. However, the Guava version of connect-common is connect.guava.version, which results in two versions of the same package in the classpath.
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/connector/connect/common/pom.xml#L50-L61
   
   Based on the above, it will lead to:
   - The server with the connect profile has version 'guava.version' of '${spark.shade.packageName}.guava'.
   - The server with the connect profile has version 'connect.guava.version' of '${spark.shade.packageName}.guava'.
   - The server with the connect profile has version 'connect.guava.version' of '${spark.shade.packageName}.connect.guava'.
   - The server with the connect profile does not have version 'connect.guava.version' of 'failureaccess'
   
   
   This PR plans to make modifications following this line of thought,  we should use connect-common shade guava, connect-client and connect-server shade connect-common to make connect-server and connect-client use the same guava dependency.
   
   



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372554800


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   The current Apache Spark includes two versions of Guava, which are (The historical reasons are not clear to me. @LuciferYang may know some background):
   1. https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/pom.xml#L203
   2. https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/pom.xml#L292
   
   The current Apache Spark has shading in the following three places for these two versions:
   1. for guava.version
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/common/network-common/pom.xml#L125-L129
   2. for connect.guava.version
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/connector/connect/server/pom.xml#L310-L316
   3. for conflicting versions, the connect-common does not override the parent's shade plugin, so it is relocated to '${spark.shade.packageName}.guava'. However, the Guava version of connect-common is connect.guava.version, which results in two versions of the same package in the classpath.
   https://github.com/apache/spark/blob/8cdcfd262f9fd46fb9a8e1ceb0bccefe452582bd/connector/connect/common/pom.xml#L56-L61
   
   This PR plans to make modifications following this line of thought,  we should use connect-common shade guava, connect-client and connect-server shade connect-common to make connect-server and connect-client use the same guava dependency.
   
   



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1426355353


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   From the error message, it seems that the connect-common module has inherited the shade plugin configuration from the parent pom.xml, and then performed shade + relocation on Guava 32.0.1-jre, but missed `com.google.guava:failureaccess`. Perhaps overwriting the configuration of the shade plugin in the connect-common module to do nothing would solve the problem?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372478531


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   So in how many places will we shade Guava now? 2? or 3? AFAICT we have the relocation defined by the parent, this one, and the one below that might come into play?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1768713962

   > Kindly ping @juliuszsompolski @LuciferYang @dongjoon-hyun , Could you please take a look if you find a moment ~
   
   I will discuss the specifics with you offline tomorrow.
   
   


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1768712034

   hmm... also cc @hvanhovell 


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1427561278


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   yea



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1772001348

   A bit busy today, I'll check this PR later.
   
   


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1372478958


##########
connector/connect/common/pom.xml:
##########
@@ -140,6 +140,35 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>

Review Comment:
   Why do we need to shade common? For the off-chance someone takes a direct dependency on it?



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error "java.lang.ClassNotFoundException: org.sparkproject.guava.util.concurrent.internal.InternalFutureFailureAccess" [spark]

Posted by "Yikf (via GitHub)" <gi...@apache.org>.
Yikf commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1768709034

   Kindly ping @juliuszsompolski @LuciferYang @dongjoon-hyun , Could you please take a look if you find a moment ~


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43436:
URL: https://github.com/apache/spark/pull/43436#discussion_r1426350579


##########
connector/connect/client/jvm/pom.xml:
##########
@@ -124,6 +124,10 @@
                 <include>io.grpc.**</include>
               </includes>
             </relocation>
+            <relocation>

Review Comment:
   > The current Apache Spark includes two versions of Guava, which are (The historical reasons are not clear to me. @LuciferYang may know some background):
   
   Sorry I missed this ping. 14.0.1 is because Hive 2.3.9 depends on Guava 14.0.1, which is shaded in the `network-common` module and used by other non-connect modules. `32.0.1-jre` is dedicated to Connect-related modules. Currently, the connect server and connect jvm client modules should have shaded it respectively, and for the Guava 32.0.1-jre, the dependency `com.google.guava:failureaccess` should also be shaded.



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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #43436: [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error
URL: https://github.com/apache/spark/pull/43436


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1897729458

   Merged into master. Thanks @Yikf @hvanhovell @c3-ffomenko 


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


Re: [PR] [SPARK-45593][BUILD] Building a runnable distribution from master code running spark-sql raise error [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43436:
URL: https://github.com/apache/spark/pull/43436#issuecomment-1894967772

   @Yikf Can you re-trigger the failed GA tasks?
   
   @hvanhovell Do you have any further suggestions for this pr? If not, I will merge it after it passes all GA tests. Thanks
   
   
   
   


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