-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[Build][Bugfix] Fix protobuf version #54910
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
Conversation
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.
Summary of Changes
Hello @MengqingCao, 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 a build-related bug by updating the protobuf dependency version. The change aims to resolve a specific AttributeError encountered when packaging messages to dictionaries, ensuring compatibility and proper functioning of protobuf-related operations.
Highlights
- Dependency Update: The
protobuflibrary dependency has been updated inpython/requirements.txtto>3.20.0. This change specifically removes the previous version constraint!=3.19.5,>=3.15.3. - Bug Fix: This dependency update is intended to resolve an
AttributeError: 'str' object has no attribute 'DESCRIPTOR'that occurs when packaging messages to dictionaries usingprotobuf.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 or fill out our survey to provide feedback.
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
-
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. ↩
|
PTAL, thx! @ruisearch42 @noemotiovon |
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.
Code Review
This pull request updates the protobuf dependency to address a packaging issue. A suggestion has been made to use a more specific protobuf version.
python/requirements.txt
Outdated
| msgpack<2.0.0,>=1.0.0 | ||
| packaging | ||
| protobuf!=3.19.5,>=3.15.3 | ||
| protobuf>3.20.0 |
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.
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.
agree with the AI, let's do >=3.20.3 since it should be only bug fixes (https://pypi.org/project/protobuf/3.20.3/)
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.
ping @MengqingCao ^
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.
oh sorry for missing that, I'll update right now.
|
LGTM! Appreciate your effort. |
### What this PR does / why we need it? Fix protobuf version in Dockerfile to resolve `AttributeError: 'str' object has no attribute 'DESCRIPTOR' when packaging message to dict` using protobuf. will remove version specification after ray-project/ray#54910 is merged ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0e36abf Signed-off-by: MengqingCao <[email protected]>
israbbani
left a comment
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.
edoakes
left a comment
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.
Good with me
edoakes
left a comment
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.
Sorry, scanned it too quickly. We also need to update setup.py (per the comment above: https://github.com/ray-project/ray/pull/54910/files#diff-b72f6e5561c432513e240838ae1eeb3f85237fa3ed5d701ad4c3be2a55c57b9dR5)
Thanks for pointing that, updated now :-) |
|
well.. if core team says it is okay, I have no objection.. this is practically firing ray users that cannot upgrade protobuf to >3.20. I wonder if there is a min reproduce script with ray (rather than through vllm) that can prove that ray (core) does not work with protobuf 3.20 or below any more. there is basically only protobuf 3.20.3 that is above 3.20.0. maybe it is better to just say protobuf>=4 now.. |
|
3.20 is from 2022 and we already required >3.15.3 |
aslonnie
left a comment
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.
defer for @edoakes for final review actions.
|
the failing gpu tests should be already fixed on master. |
Thanks for this info, I have rebased the code to the latest master now |
Signed-off-by: MengqingCao <[email protected]>
|
It seems CI failed due to network issue, could you help retrigger this? @edoakes |
Fix protobuf version in Dockerfile to resolve `AttributeError: 'str' object has no attribute 'DESCRIPTOR' when packaging message to dict` using protobuf. will remove version specification after ray-project/ray#54910 is merged N/A CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0e36abf Signed-off-by: MengqingCao <[email protected]>
Done. You can always re-trigger by merging latest master and pushing as well. |
|
Got it, thanks!
…---- Replied Message ----
| From | Edward ***@***.***> |
| Date | 08/11/2025 23:19 |
| To | ***@***.***> |
| Cc | Mengqing ***@***.***>***@***.***> |
| Subject | Re: [ray-project/ray] [Build][Bugfix] Fix protobuf version (PR #54910) |
edoakes left a comment (ray-project/ray#54910)
It seems CI failed due to network issue, could you help retrigger this? @edoakes
Done. You can always re-trigger by merging latest master and pushing as well.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This pr fixes the `AttributeError: 'str' object has no attribute 'DESCRIPTOR'` when packaging message to dict using `protobuf` Closes #54849 Signed-off-by: MengqingCao <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: sampan <[email protected]>
### What this PR does / why we need it? Fix protobuf version in Dockerfile to resolve `AttributeError: 'str' object has no attribute 'DESCRIPTOR' when packaging message to dict` using protobuf. will remove version specification after ray-project/ray#54910 is merged backport of #2028 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with new added test. --------- Signed-off-by: MengqingCao <[email protected]>
This pr fixes the `AttributeError: 'str' object has no attribute 'DESCRIPTOR'` when packaging message to dict using `protobuf` Closes ray-project#54849 Signed-off-by: MengqingCao <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Andrew Grosser <[email protected]>
This pr fixes the `AttributeError: 'str' object has no attribute 'DESCRIPTOR'` when packaging message to dict using `protobuf` Closes ray-project#54849 Signed-off-by: MengqingCao <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: jugalshah291 <[email protected]>
### What this PR does / why we need it? Fix protobuf version in Dockerfile to resolve `AttributeError: 'str' object has no attribute 'DESCRIPTOR' when packaging message to dict` using protobuf. will remove version specification after ray-project/ray#54910 is merged ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0e36abf Signed-off-by: MengqingCao <[email protected]>
This pr fixes the `AttributeError: 'str' object has no attribute 'DESCRIPTOR'` when packaging message to dict using `protobuf` Closes ray-project#54849 Signed-off-by: MengqingCao <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Douglas Strodtman <[email protected]>
### What this PR does / why we need it? Fix protobuf version in Dockerfile to resolve `AttributeError: 'str' object has no attribute 'DESCRIPTOR' when packaging message to dict` using protobuf. will remove version specification after ray-project/ray#54910 is merged ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with existing test. - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@0e36abf Signed-off-by: MengqingCao <[email protected]>

Why are these changes needed?
This pr fixes the
AttributeError: 'str' object has no attribute 'DESCRIPTOR'when packaging message to dict usingprotobufRelated issue number
Closes #54849
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.