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

Fix aiohttp 3.8.0 breaking changes (and unpin from 3.7) #20261

Merged
merged 5 commits into from Nov 11, 2021
Merged

Fix aiohttp 3.8.0 breaking changes (and unpin from 3.7) #20261

merged 5 commits into from Nov 11, 2021

Conversation

TeoZosa
Copy link
Contributor

@TeoZosa TeoZosa commented Nov 11, 2021 •
edited

Fixes compatibility with aiohttp >= 3.8

Why are these changes needed?

As of version 3.8.0, aiohttp uses external frozenlist & aiosignal dependencies and no longer provides users internal variants.

While #19948 temporarily fixes this, this PR resolves the longer-term issue (viz. #19951) of aiohttp compatibility.

Related issue number

Checks

  • 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 :(

TeoZosa added a commit to TeoZosa/ray-summit-demo-2021-public that referenced this pull request Nov 11, 2021
@TeoZosa
🔧 Fix incompatible dependencies specification
14988ca 
Note: manually patching installed `ray` Python source code to fix
broken dashboard functionality (see ray-project/ray#20261).
simon-mo
simon-mo approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

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

LGTM! For TLs, this PR adds aiosignal and frozenlist to the ray[default] deps. They are about 200Kb total.

@simon-mo simon-mo added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Nov 11, 2021
@simon-mo simon-mo merged commit abf0eb5 into ray-project:master Nov 11, 2021
@simon-mo
Copy link
Contributor

Thank you @TeoZosa

@pcmoritz pcmoritz mentioned this pull request Nov 12, 2021
Merged
6 tasks
@TeoZosa
Copy link
Contributor Author

TeoZosa commented Nov 12, 2021

Thank you @TeoZosa

My pleasure! 🤙🏻 Thank you @simon-mo and the rest of the team for the quick turnaround on this, I hope you all have a great week!

This was referenced Nov 12, 2021
Closed
Closed
@marcm-ml marcm-ml mentioned this pull request Nov 16, 2021
Closed
2 tasks
@pcmoritz pcmoritz mentioned this pull request Nov 24, 2021
Closed
2 tasks
@aviau
Copy link

aviau commented Dec 13, 2021

closes #20681 ?

@jjyao jjyao mentioned this pull request Jan 13, 2022
Merged
5 tasks
This was referenced Feb 8, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edoakes edoakes edoakes approved these changes

@simon-mo simon-mo simon-mo approved these changes

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

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

Assignees

@simon-mo simon-mo

Labels
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.

Unpin aiohttp dependency
4 participants
@TeoZosa @simon-mo @aviau @edoakes