Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce gRPC dependency footprint, get rid of duplicate classes #1365

Merged

Conversation

mziccard
Copy link
Contributor

@mziccard mziccard commented Nov 2, 2016

This reduced gRPC dependencies that we include (in particular in the core module) fixes #1278 for good.

@anthmgoogle @garrettjonesgoogle any chance you can have a look? This can also be done at GAX layer to fix googleapis/gax-java#133. Also, is there a reason why proto artifacts (as grpc-google-iam-v1 or grpc-google-pubsub-v1) depend on the whole grpc-all package? I believe that the dependency list there can be heavily reduced.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 2, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 83.896% when pulling d9d23ba on mziccard:reduce-grpc-dependency-footprint into a963ef4 on GoogleCloudPlatform:master.

@garrettjonesgoogle
Copy link
Member

Maybe I should bump the priority of the dependency change in gax so that you don't have to make this mess.

@mziccard
Copy link
Contributor Author

mziccard commented Nov 2, 2016

Actually GAX is not a big issue. We use GAX in our google-cloud-core module, but even with the GAX issue fixed I would still want to exclude some of its gRPC dependency (namely all but grpc-protobuf) ingoogle-cloud-core (to reduce its footprint for those modules that depende on it but don't use gRPC). Then in gRPC-based modules I would still have to re-add all grpc dependencies that I previously excluded.

What is causing this mess is the fact that all proto artifacts depend on grpc-all which is as surprising as unnecessary.

@garrettjonesgoogle
Copy link
Member

Ahh, the proto artifacts - how much would it help for me to update their dependencies too?

@mziccard
Copy link
Contributor Author

mziccard commented Nov 2, 2016

Ahh, the proto artifacts - how much would it help for me to update their dependencies too?

My guess is that they only need grpc-protubuf, if this is really the case I can get rid of all <exclude> elements for them (there's one for each pom.xml in this PR and 2 in the core module's pom.xml).

It's not a problem to send out this PR and then remove the <excludes>. Releasing releasing new proto artifacts for all services might take you some time.

@@ -104,19 +104,42 @@
<version>3.0.0</version>
</dependency>
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-protobuf</artifactId>
<version>1.0.1</version>

This comment was marked as spam.

This comment was marked as spam.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 83.896% when pulling 6be9e2c on mziccard:reduce-grpc-dependency-footprint into a963ef4 on GoogleCloudPlatform:master.

@garrettjonesgoogle
Copy link
Member

LGTM

@mziccard
Copy link
Contributor Author

mziccard commented Nov 3, 2016

@garrettjonesgoogle thanks for reviewing!

@mziccard mziccard merged commit 09ad52a into googleapis:master Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cherry-pick gRPC dependencies rather than using grpc-all Datastore: duplicate classes in dependencies
4 participants