Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
mziccard merged 2 commits into from
Nov 4, 2016
Merged

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

mziccard merged 2 commits into from
Nov 4, 2016

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

coveralls commented Nov 2, 2016

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

garrettjonesgoogle commented Nov 2, 2016

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

garrettjonesgoogle commented Nov 2, 2016

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.

garrettjonesgoogle
garrettjonesgoogle reviewed Nov 2, 2016
View reviewed changes
google-cloud-core/pom.xml
</dependency>
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-protobuf</artifactId>

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

coveralls commented Nov 3, 2016

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

garrettjonesgoogle commented Nov 3, 2016

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
meltsufin pushed a commit that referenced this pull request Dec 22, 2025
@renovate-bot
build(deps): update dependency org.apache.maven.plugins:maven-surefir…
7e55872 
…e-plugin to v3.1.2 (#1365)
chingor13 pushed a commit that referenced this pull request Jan 6, 2026
@release-please
chore(main): release 0.131.12-alpha-SNAPSHOT (#1365)
a16a4b4 
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@garrettjonesgoogle garrettjonesgoogle garrettjonesgoogle left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

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

@mziccard @coveralls @garrettjonesgoogle @googlebot