Conversation
Signed-off-by: jeremyhi <fengjiachun@gmail.com>
Summary of ChangesHello @fengjiachun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant dependency update, primarily upgrading the gRPC framework to its latest stable version (1.78.0) and the Mockito testing library to version 4.11.0. These updates ensure the project benefits from the latest features, performance enhancements, and security fixes provided by these critical libraries. The changes involve adjusting Maven Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request upgrades the gRPC version to 1.78.0 and Mockito to 4.11.0. The changes primarily involve updating pom.xml files to reflect the new versions and adjust dependencies. The required code change for the Mockito upgrade in a test file is also correctly handled. My review focuses on the dependency management in ingester-bulk-protocol/pom.xml, where I've found a critical issue with conflicting gRPC transport dependencies and a redundant dependency declaration that affects maintainability.
| <dependency> | ||
| <groupId>io.grpc</groupId> | ||
| <artifactId>grpc-netty</artifactId> | ||
| <version>${io.grpc.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
This dependency on grpc-netty conflicts with grpc-netty-shaded which is transitively included from the ingester-grpc module. Having both on the classpath is problematic and can lead to LinkageErrors at runtime. Please remove this dependency and rely on the grpc-netty-shaded provided by ingester-grpc for the gRPC transport.
| <dependency> | ||
| <groupId>io.grpc</groupId> | ||
| <artifactId>grpc-api</artifactId> | ||
| <version>${io.grpc.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
This explicit dependency on grpc-api is redundant. It is already provided as a transitive dependency by other grpc-* artifacts (like grpc-stub) from the ingester-grpc module. To improve maintainability and simplify the POM, it's better to remove this declaration and rely on Maven's transitive dependency resolution.
As @copilot said