tests: Add protobuf 3.x compatibility tests with a third party library tensorflow #4082
Conversation
…y tensorflow. (#4015) Add protobuf 3.x compatibility tests with a third party library tensorflow in showcase module. Tensorflow depends on [protobuf-java v3.21.9](https://github.com/tensorflow/java/blob/cbf942051d55291ba9bdb019b2f207f013889bba/tensorflow-core/pom.xml#L45) gen code and runtime. This proves that client libraries with 4.33 runtime and gen code are compatible with a third party library that contains 3.x gen code and runtime. --------- Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
Summary of ChangesHello @blakeli0, 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 introduces a new set of integration tests designed to validate the backward compatibility of the project's current Protobuf 4.x implementation with older Protobuf 3.x versions. By integrating TensorFlow, which uses Protobuf 3.x, the changes demonstrate that client libraries built with Protobuf 4.x can seamlessly interact with systems and data structures relying on Protobuf 3.x, ensuring broader ecosystem compatibility. Highlights
Ignored Files
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new compatibility test to ensure that client libraries using Protobuf 4.x are compatible with third-party libraries, like TensorFlow, that depend on Protobuf 3.x. The changes include adding the TensorFlow dependency, creating a new test case, and configuring Maven profiles to run this test in isolation. The implementation is sound, but I have one suggestion to improve the robustness of the new test code by handling potential exceptions more gracefully.
| private static Integer getValueFromGraphDefByName(GraphDef graphDef, String name1) { | ||
| return graphDef.getNodeList().stream() | ||
| .filter(nodeDef -> nodeDef.getName().equals(name1)) | ||
| .findFirst() | ||
| .get() | ||
| .getAttrOrThrow("value") | ||
| .getTensor() | ||
| .getIntValList() | ||
| .get(0); | ||
| } |
There was a problem hiding this comment.
The current implementation of getValueFromGraphDefByName uses .get() on an Optional and .get(0) on a List without checking for presence or emptiness. This can lead to NoSuchElementException or IndexOutOfBoundsException if the expected node or value is not found, which can make debugging test failures difficult. Using orElseThrow with a descriptive message would provide more informative error messages.
Additionally, the parameter name name1 is a bit specific for a general-purpose helper method. Renaming it to name would improve clarity.
I've suggested a change that addresses both points, making the method more robust and readable.
private static Integer getValueFromGraphDefByName(GraphDef graphDef, String name) {
return graphDef.getNodeList().stream()
.filter(nodeDef -> nodeDef.getName().equals(name))
.findFirst()
.orElseThrow(() -> new AssertionError("Node not found: " + name))
.getAttrOrThrow("value")
.getTensor()
.getIntValList()
.stream()
.findFirst()
.orElseThrow(() -> new AssertionError("Node has no int value: " + name));
}
|
|
2 participants
Add protobuf 3.x compatibility tests with a third party library tensorflow in showcase module.
Tensorflow depends on protobuf-java
v3.21.9 gen code and runtime. This proves that client libraries with 4.33 runtime and gen code are compatible with a third party library that contains 3.x gen code and runtime.