Rethink Guava dependency in Java driver#1983
Rethink Guava dependency in Java driver#1983lukasz-antoniak wants to merge 3 commits intoapache:4.xfrom
Conversation
There was a problem hiding this comment.
Manually verified the tar.gz, looks good.
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
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 <adutra@users.noreply.github.com>
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.