Rethink Guava dependency in Java driver#1983
Rethink Guava dependency in Java driver#1983lukasz-antoniak wants to merge 3 commits intoapache:4.xfrom
Conversation
| <exclude>org.apache.cassandra:java-driver-core</exclude> | ||
| <exclude>org.apache.cassandra:java-driver-mapper-runtime</exclude> | ||
| <exclude>org.apache.cassandra:java-driver-mapper-processor</exclude> | ||
| <exclude>org.apache.cassandra:java-driver-guava-shaded</exclude> |
There was a problem hiding this comment.
Manually verified the tar.gz, looks good.
| limitations under the License. | ||
|
|
||
| --> | ||
| <assembly xmlns="http://maven.apache.org/ASSEMBLY/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.0.0 http://maven.apache.org/xsd/assembly-2.0.0.xsd"> |
There was a problem hiding this comment.
Manually inspected the shaded jar, also looking good.
508ac2f to
3ef49df
Compare
emerkle826
left a comment
There was a problem hiding this comment.
This looks good to me
|
So, I think this is fine as it stands, and it appears to accomplish the goal we want but I can't help but wonder... do we really even need this JAR anymore? Is there a clear argument for including this shaded JAR vs. just including (as a shaded JAR or as a stated dependency) Guava directly? The above may be a dumb question; it's quite possible there's some aspect of this I'm not seeing. But as I look through this code I can't help but think that we're going to a lot of trouble to create a dependency from a shaded Guava JAR which seems... like something we could do without all that trouble. |
bom/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.apache.cassandra</groupId> | ||
| <artifactId>java-driver-guava-shaded</artifactId> | ||
| <version>4.18.2-SNAPSHOT</version> |
There was a problem hiding this comment.
So we're versioning the shaded JAR based on... the driver release, which means we need to release a new version with every driver release... whether we've upgraded Guava or not? This seems... strange.
|
Based on conversation in ASF Slack it sounds like this might be moving in the direction of adding Guava to core-shaded and just making Guava a straight dependency for the non-shaded build. This creates two very distinct configurations: one with zero shading (the core JAR) and one with multiple major dependencies shaded (core-shaded). If users experience conflicts with the Guava version in use in their application and what the Java driver needs we can recommend those users move to core-shaded. The analysis above is not settled yet, however, so this could still change. |
patch by Lukasz Antoniak; reviewed by Alexandre Dutra and Erik Merkle for CASSJAVA-52 Update pom.xml Co-authored-by: Alexandre Dutra <[email protected]>
93b4c1d to
7641f18
Compare
|
Declared Guava as a dependency to core module. Changes pushed. @absurdfarce, @adutra: I still have a problem with |
|
Checking if TinyBundles will do the job. |
|
I ended up implementing |
668b9d1 to
d478294
Compare
|
I am changing the title of this PR to later rethink handling he dependency of Guava in the Java driver. All details have been captured in CASSJAVA-62. Let us wait with this PR until we are allowed to do a potentially breaking change. This PR includes some valuable work to use Guava as standard dependency (mainly one OSGi test). |
Fixes CASSJAVA-62.