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

Add grpc-services, grpc-utils to vendored grpc #30196

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

arunpandianp
Copy link
Contributor

grpc-services is needed to get Channelz information #24835. grpc-utils is a dependency of grpc-services

checkJavaLinkage didn't report any new failures.

@arunpandianp
Copy link
Contributor Author

R: @Abacn

Copy link
Contributor

github-actions bot commented Feb 2, 2024

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@Abacn
Copy link
Contributor

Abacn commented Feb 2, 2024

The current vendor process does not afford frequent change. If there are recurring changes then it needs to use unvendored grpc, or we unvendor grpc at all

CC: @kennknowles

@kennknowles
Copy link
Member

The main reason for vendoring is that the things we vendor have frequent backwards-incompatible and forward-incompatible changes and are widely used so that they often cause conflicts with IOs and user pipelines. Perhaps we can remain vendored while decreasing toil but that would require some cleverness. I think adding more related libraries to a massive vendor uberjar so we still only have one "release" could be possible.

@kennknowles
Copy link
Member

FWIW I think this change is OK but it is some work to then re-release the vendored artifact. So using today's processes you just have to budget for it.

@arunpandianp
Copy link
Contributor Author

Tested the upgrade here #30212. Let me know if anything else is required for merging this.

Copy link
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@Abacn Abacn merged commit 78d0823 into apache:master Feb 6, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants