-
Notifications
You must be signed in to change notification settings - Fork 609
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
Bump up grpc #650
Bump up grpc #650
Conversation
rocketmq plugin is preventing us from upgrading grpc now |
Could you recheck the CI? |
👆 |
What do you mean? How a plugin blocks a lib upgrade? |
The log is better than hundreds of words:
https://github.com/apache/skywalking-java/actions/runs/7015220764/job/19084175721?pr=650#step:3:431 |
rocketmq sets a hard requirement of gRPC 1.53.0, upgrading our core grpc to >1.53.0 breaks maven dependency resolution |
<exclusions> | ||
<exclusion> | ||
<groupId>io.grpc</groupId> | ||
<artifactId>grpc-netty</artifactId> | ||
</exclusion> | ||
</exclusions> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should exclude grpc from agent-core, rather than from this side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? GRPC is needed in agent side, how can we exclude it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the agent plugin depends on the agent core lib, which has the gRPC dependency overriding the version.
We just need the RocketMQ plugin compiled. So, about exclude, I mean for this plugin, it could depend on agent-core with gRPC excluded. In the runtime, this plugin is using the user's gRPC, our gRPC and netty would have been shaded in another package already. So, we should be good by that.
<dependency> | ||
<groupId>org.apache.skywalking</groupId> | ||
<artifactId>apm-agent-core</artifactId> | ||
<version>${project.version}</version> | ||
<scope>provided</scope> | ||
<exclusions> | ||
<exclusion> | ||
<groupId>io.grpc</groupId> | ||
<artifactId>grpc-netty</artifactId> | ||
</exclusion> | ||
</exclusions> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are my new changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dep upgrade works.
CHANGES
log.