Skip to content

feat: upgrade gRPC version to 1.78#100

Open
fengjiachun wants to merge 1 commit intomainfrom
feat/grpc-upgrade
Open

feat: upgrade gRPC version to 1.78#100
fengjiachun wants to merge 1 commit intomainfrom
feat/grpc-upgrade

Conversation

@fengjiachun
Copy link
Contributor

As @copilot said

Signed-off-by: jeremyhi <fengjiachun@gmail.com>
@gemini-code-assist
Copy link

Summary of Changes

Hello @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 pom.xml files across multiple modules to reflect the new versions and adapt to updated dependency management practices, such as migrating from mockito-all to mockito-core and updating test runner imports.

Highlights

  • gRPC Version Upgrade: The gRPC library has been upgraded to version 1.78.0, centralizing its version definition in the root pom.xml and adjusting module-specific dependencies to align with this new version.
  • Mockito Library Update: The Mockito testing framework has been updated to version 4.11.0. This includes replacing the deprecated mockito-all artifact with mockito-core across various modules and updating the MockitoJUnitRunner import path in test files.
  • Dependency Management Refinement: Dependency declarations have been refined, including adding explicit grpc-api and grpc-netty dependencies in ingester-bulk-protocol and removing a redundant io.grpc.version property from ingester-grpc.
Changelog
  • ingester-bulk-protocol/pom.xml
    • Added an exclusion for grpc-api within the grpc-protobuf dependency.
    • Explicitly added grpc-api and grpc-netty as direct dependencies.
    • Replaced mockito-all artifact with mockito-core.
  • ingester-grpc/pom.xml
    • Removed the local io.grpc.version property, now inherited from the parent pom.xml.
  • ingester-protocol/pom.xml
    • Replaced mockito-all artifact with mockito-core.
  • ingester-protocol/src/test/java/io/greptime/WriteClientTest.java
    • Updated the import for Mockito's JUnit runner from org.mockito.runners.MockitoJUnitRunner to org.mockito.junit.MockitoJUnitRunner.
  • ingester-rpc/pom.xml
    • Replaced mockito-all artifact with mockito-core.
  • pom.xml
    • Updated io.grpc.version property to "1.78.0".
    • Updated mockito.version property to "4.11.0".
    • Replaced mockito-all artifact with mockito-core in the dependency management section.
Activity
  • No specific activity (comments, reviews, etc.) has been recorded for this pull request yet.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@fengjiachun
Copy link
Contributor Author

@codex review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +79 to +83
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-netty</artifactId>
<version>${io.grpc.version}</version>
</dependency>

Choose a reason for hiding this comment

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

critical

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.

Comment on lines +74 to +78
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-api</artifactId>
<version>${io.grpc.version}</version>
</dependency>

Choose a reason for hiding this comment

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

medium

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant