ci: revert to grpc v1.71.2 to reduce test flakiness#16027
ci: revert to grpc v1.71.2 to reduce test flakiness#16027scotthart wants to merge 1 commit intogoogleapis:mainfrom
Conversation
Summary of ChangesHello, 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 addresses issues with test flakiness by rolling back the gRPC library version across multiple build environments and installation instructions. The update ensures that development and CI processes utilize a more stable gRPC release, which is expected to improve the reliability of automated tests. 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
|
![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 reverts the gRPC version from v1.76.0 to v1.71.2 across multiple Dockerfiles and documentation. While the changes are straightforward, I've noted that the gRPC version is hardcoded in many places. To improve maintainability and simplify future updates, I've suggested using variables in the Dockerfiles and placeholders in the documentation to avoid this duplication, aligning with the repository's style guide.
| # ```bash | ||
| WORKDIR /var/tmp/build/grpc | ||
| RUN curl -fsSL https://github.com/grpc/grpc/archive/v1.76.0.tar.gz | \ | ||
| RUN curl -fsSL https://github.com/grpc/grpc/archive/v1.71.2.tar.gz | \ |
There was a problem hiding this comment.
To improve maintainability and align with the repository's style guide, consider defining the gRPC version in an ARG. The version string is duplicated across many Dockerfiles, making updates tedious and error-prone.
For example:
- Add
ARG GRPC_VERSION=v1.71.2at the top of the file. - Change this line to use the variable:
RUN curl -fsSL https://github.com/grpc/grpc/archive/${GRPC_VERSION}.tar.gz | \This would make future version updates much simpler.
References
- The repository style guide states a preference for factoring out duplicated code when it appears 3 or more times in non-test files. This is relevant here as the gRPC version string is duplicated across many Dockerfiles. (link)
There was a problem hiding this comment.
+1 to this, although not in scope of this PR.
| ```bash | ||
| mkdir -p $HOME/Downloads/grpc && cd $HOME/Downloads/grpc | ||
| curl -fsSL https://github.com/grpc/grpc/archive/v1.76.0.tar.gz | \ | ||
| curl -fsSL https://github.com/grpc/grpc/archive/v1.71.2.tar.gz | \ |
There was a problem hiding this comment.
Similar to the Dockerfiles, the gRPC version is hardcoded multiple times in this documentation file. This can lead to inconsistencies if a version is missed during an update. Consider using a placeholder for the version number (e.g., {{GRPC_VERSION}}) and a script to substitute it with the correct version. This would ensure consistency and simplify future updates.
References
- The repository style guide states a preference for factoring out duplicated code when it appears 3 or more times in non-test files. This is relevant here as the gRPC version string is duplicated multiple times within this documentation file. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16027 +/- ##
=======================================
Coverage 92.63% 92.63%
=======================================
Files 2335 2335
Lines 214709 214709
=======================================
+ Hits 198905 198906 +1
+ Misses 15804 15803 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
diegomarquezp
left a comment
diegomarquezp
left a comment
There was a problem hiding this comment.
I see the Dockerfiles' gRPC versions were updated.
In which CIs do we expect to see less flakiness?
| # ```bash | ||
| WORKDIR /var/tmp/build/grpc | ||
| RUN curl -fsSL https://github.com/grpc/grpc/archive/v1.76.0.tar.gz | \ | ||
| RUN curl -fsSL https://github.com/grpc/grpc/archive/v1.71.2.tar.gz | \ |
There was a problem hiding this comment.
+1 to this, although not in scope of this PR.
A lot of the demo-* builds were encountering a "double free" memory error when executing quickstarts. It appears to be some type of issue between grpc and openssl as the program is exiting. |
Wouldn't this be a good thing to say in the PR description, instead of just "reduce flakiness" in the title? Not every PR deserves a description, but it looks like I currently have to go back three months (~80 PRs) before I find a PR that has one, and then another month before finding the next one. That seems sparse. |
3 participants
No description provided.