Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Core][Protobuf] loose upper bound for protobuf #29224

Merged
merged 3 commits into from
Oct 17, 2022
Merged

[Core][Protobuf] loose upper bound for protobuf #29224

merged 3 commits into from
Oct 17, 2022

Conversation

scv119
Copy link
Contributor

@scv119 scv119 commented Oct 11, 2022 •
edited
Loading

Why are these changes needed?

This upper bound was introduced in #25211 but the root cause is supposed be fixed by #25648.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@scv119 scv119 marked this pull request as ready for review October 11, 2022 19:53
@scv119 scv119 requested a review from fishbone October 11, 2022 19:53
@scv119 scv119 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 11, 2022
rkooo567
rkooo567 approved these changes Oct 13, 2022
View reviewed changes
Copy link
Contributor

@rkooo567 rkooo567 left a comment

Choose a reason for hiding this comment

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

How can we test this btw?

@rkooo567
Copy link
Contributor

is there a test case that broke before?

@scv119 scv119 added the release-blocker P0 Issue that blocks the release label Oct 13, 2022
@scv119 scv119 added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Oct 17, 2022
@scv119
Copy link
Contributor Author

scv119 commented Oct 17, 2022

@rkooo567 we have test_latest_core_dependencies which test the latest protobuf is compatible with ray.

@scv119 scv119 merged commit 37c2b58 into ray-project:master Oct 17, 2022
scv119 added a commit to scv119/ray that referenced this pull request Oct 17, 2022
@scv119
[Core][Protobuf] loose upper bound for protobuf (ray-project#29224)
342642b 
This upper bound was introduced in ray-project#25211 but the root cause is supposed be fixed by ray-project#25648.
rickyyx pushed a commit that referenced this pull request Oct 17, 2022
@scv119
[Core][Protobuf] loose upper bound for protobuf (#29224) (#29409)
a1b5d7e 
This upper bound was introduced in #25211 but the root cause is supposed be fixed by #25648.
@krfricke krfricke mentioned this pull request Oct 23, 2022
Merged
7 tasks
krfricke added a commit that referenced this pull request Oct 24, 2022
@krfricke
[ci] Remove protobuf pin to < 4 (repeat) (#29584)
f021621 
#29224 removed the protobuf upper bound but #29375 was oroginally created on an older requirements file and accidentally reverted the update for unix builds.

Signed-off-by: Kai Fricke <[email protected]>
@ascillitoe ascillitoe mentioned this pull request Nov 8, 2022
Merged
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
@scv119 @WeichenXu123
[Core][Protobuf] loose upper bound for protobuf (ray-project#29224)
68b879e 
This upper bound was introduced in ray-project#25211 but the root cause is supposed be fixed by ray-project#25648.

Signed-off-by: Weichen Xu <[email protected]>
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
@krfricke @WeichenXu123
[ci] Remove protobuf pin to < 4 (repeat) (ray-project#29584)
70eac4d 
ray-project#29224 removed the protobuf upper bound but ray-project#29375 was oroginally created on an older requirements file and accidentally reverted the update for unix builds.

Signed-off-by: Kai Fricke <[email protected]>
Signed-off-by: Weichen Xu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkooo567 rkooo567 rkooo567 approved these changes

@richardliaw richardliaw richardliaw approved these changes

@jjyao jjyao jjyao approved these changes

@fishbone fishbone Awaiting requested review from fishbone

@ericl ericl Awaiting requested review from ericl

@edoakes edoakes Awaiting requested review from edoakes edoakes is a code owner

Assignees

@rkooo567 rkooo567

@fishbone fishbone

Labels
release-blocker P0 Issue that blocks the release tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@scv119 @rkooo567 @jjyao @richardliaw @fishbone