Would it be possible to shade guava? #2688
Comments
|
Case and point grpc 1.1.x Isn't compatible with datastax cassandra driver 3.x. |
|
@GEverding Looks like datastax is making progress on this. apache/cassandra-java-driver#784 |
|
We've discussed possibly shading but excluding ListenableFuture (and maybe SettableFuture), so they still come from Guava. But in the short term we're more likely to go back to using Guava 19 like we were in v1.0.x which should restore support with older versions of Guava. |
|
And yes, we've heard specific issue with Cassandra. We've also seen https://datastax-oss.atlassian.net/browse/JAVA-1328 |
If we are also going to keep Guava 19 for further releases, we need to ask Census/instrumentation to downgrade to 19 too. Their previous releases have been using 19, but they have bumped to 20 on their master. @sebright |
|
@ejona86 Sticking with Guava 19 sounds fine. Do you plan to upgrade to Guava 22 with its compatibility layer once it's release? What about the option to no longer expose Guava in the stubs and generated code? Would you consider having a generator option to disable the ListenableFuture stubs and another option to generate CompletableFuture stubs? I'd be more than happy to submit a PR for that but want to make sure you think this approach makes sense. |
|
@zhangkun83 I made a PR to build instrumentation-java with guava 19.0 again: census-instrumentation/opencensus-java#99 |
|
I'd like to point out that if gRPC shades all usages of Guava except ListenableFuture, there probably is no need to remove the ListenableFuture reference. ListenableFuture is non-Beta, unlikely to change, and as an interface any incompatible change would have to be given a new name which allows for more compatibility tricks. That would be compatible with every version of Guava (all the way back to 1.0!). Note that this isn't trivial, because of the many different artifacts gRPC has. And it has the downfall of increasing code bloat on platforms like Android. But dealing with the stub is probably unnecessary.
Uncertain. If we bumped soon, it may require us to shade. We're in talks internally to figure out how to handle Guava.
While possible, it doesn't address as much as you may hope. First, to prevent class collisions the generated code must be canonical, and so it would need to use a different class name. Second, it doesn't help API publishes, who like Google may provide pre-built packages (see group id com.google.api.grpc on Maven Central). And this is a bit more complex since the Future stub is API-stable, so while it may solve the issue for you, it doesn't necessarily solve the wider community.
Possible, but there'd probably be argumentation first. I'm very disappointed in it and the more I talk to people the more it seems the community has decided that CompletableFuture is horrible. Some problems could be overlooked, but the list is so long... Don't want to get into it here though. If there's interest, we can open another issue to discuss. And adding any new stub still has the problem that "everyone gets it." Yes, we can have an option that enables the generation, but it'd have to be off by default and publishers wouldn't probably pre-generate it. Java's fragmentation really hurts here. |
Guava 20 introduced some overloading optimizations for Preconditions that require using Guava 20+ at runtime. Unfortunately, Guava 20 removes some things that is causing incompatibilities with other libraries, like Cassandra. While the incompatibility did trigger some of those libraries to improve compatibility for newer Guavas, we'd like to give the community more time to work through it. See grpc#2688 At this commit, we appear to be compatible with Guava 18+. It's not clear if we want to actually "support" 18, but it did compile. Guava 17 doesn't have at least MoreObjects, directExecutor, and firstNotNull. Guava 21 compiles without warnings, so it should be compatible with Guava 22 when it is released. One test method will fail with the upcoming Guava 22, but this won't impact applications. I made MoreThrowables to avoid using any known-deprecated Guava methods in our JARs, to reduce pain for those stuck with old versions of gRPC in the future (July 2018). In the stand-alone Android apps I removed unnecessary explicit deps instead of syncing the version used.
|
I've sent out a PR to downgrade to Guava 19 while maintaining compatibility with future versions. Assuming it is merged, we'll backport it to v1.1.x and do another release to ease the pressure here. |
Guava 20 introduced some overloading optimizations for Preconditions that require using Guava 20+ at runtime. Unfortunately, Guava 20 removes some things that is causing incompatibilities with other libraries, like Cassandra. While the incompatibility did trigger some of those libraries to improve compatibility for newer Guavas, we'd like to give the community more time to work through it. See #2688 At this commit, we appear to be compatible with Guava 18+. It's not clear if we want to actually "support" 18, but it did compile. Guava 17 doesn't have at least MoreObjects, directExecutor, and firstNotNull. Guava 21 compiles without warnings, so it should be compatible with Guava 22 when it is released. One test method will fail with the upcoming Guava 22, but this won't impact applications. I made MoreThrowables to avoid using any known-deprecated Guava methods in our JARs, to reduce pain for those stuck with old versions of gRPC in the future (July 2018). In the stand-alone Android apps I removed unnecessary explicit deps instead of syncing the version used.
|
While shading Guava (or parts of Guava) could still be a discussion point, this issue was talking about pains of Guava 20. We downgraded a long time ago to Guava 19 to help ease those and provide time for dependencies update to new Guavas. So the main issue here was resolved in a different way. |
|
Is there a different issue regarding removing/shading Guava? I would like to see this be mostly dependency-less (besides other gRPC dependencies). I know (at least from the docs) that Java 7+ is required. Java 7 reached EOL in April 2015 and Java 8 is EOL January 2019 so switching the |
Please answer these questions before submitting your issue.
What version of gRPC are you using?
1.0.3 and 1.1.1
What JVM are you using (
java -version)?java version "1.8.0_92"
Java(TM) SE Runtime Environment (build 1.8.0_92-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)
What did you do?
GRPC 1.1+ uses Guava 20.0. We have a very large codebase that uses both internal and external libraries that depend on older version of guava. These libraries are not compatible with Guava 20.0. As a result we are unable to upgrade to GRPC 1.1.+. From what I can tell the only Guava class exposed by GRPC is ListenableFutures, via ClientCalls in grpc-stub. Would it be possible to shade all other uses of Guava in GRPC.
There would still be an issue with the GRPC code generator that does expose Guava's ListenableFuture. For that use case would it be possible to make generating those stubs optional and perhaps even add an option to generate a CompletableFuture stub?
What did you expect to see?
Newer versions of GRPC to not break other libraries depending on older version of Guava.
What did you see instead?
The text was updated successfully, but these errors were encountered: