Skip to content

[serve] Unpin fastapi version#43058

Merged
edoakes merged 8 commits intoray-project:masterfrom
edoakes:unpin-fastapi
Feb 12, 2024
Merged

[serve] Unpin fastapi version#43058
edoakes merged 8 commits intoray-project:masterfrom
edoakes:unpin-fastapi

Conversation

@edoakes
Copy link
Collaborator

@edoakes edoakes commented Feb 8, 2024 •
edited
Loading

Why are these changes needed?

FastAPI has fixed the broken starlette interaction in 0.109.2: https://fastapi.tiangolo.com/release-notes/#01092

Starlette handling of root_path has changed in 0.33.0: Kludex/starlette#2352. Prior to this, it was incorrectly stripping the root_path from the path field (which we need to mirror). Added compatibility logic to handle both of these.

Related issue number

Closes #42711

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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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 :(

@edoakes
fix
b0fab5d 
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes
fix
f03643e 
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes
fix
c5031c9 
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes changed the title [serve] Unpin fastapi version [WIP][serve] Unpin fastapi version Feb 8, 2024
@edoakes
fix
3466714 
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes changed the title [WIP][serve] Unpin fastapi version [serve] Unpin fastapi version Feb 9, 2024
@edoakes edoakes requested a review from a team February 9, 2024 19:24
GeneDer
GeneDer approved these changes Feb 10, 2024
View reviewed changes
Copy link
Member

@GeneDer GeneDer left a comment

Choose a reason for hiding this comment

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

:shipit:

@edoakes edoakes mentioned this pull request Feb 12, 2024
Closed
@aslonnie
Copy link
Collaborator

aslonnie commented Feb 12, 2024

I am updating your branch to rerun the premerge tests. the test failure of test_update_task seems unrelated to the "min install" failures.

@edoakes
Copy link
Collaborator Author

edoakes commented Feb 12, 2024

I am updating your branch to rerun the premerge tests. the test failure of test_update_task seems unrelated to the "min install" failures.

Thank you!!!

@aslonnie
Copy link
Collaborator

aslonnie commented Feb 12, 2024

looks like all tests passed; should be good to merge.

I will continue investigating the min install thing

@edoakes edoakes merged commit ca88ce7 into ray-project:master Feb 12, 2024
@edoakes edoakes self-assigned this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richardliaw richardliaw richardliaw approved these changes

@GeneDer GeneDer GeneDer approved these changes

@ericl ericl Awaiting requested review from ericl

Assignees

@edoakes edoakes

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Serve] Fastapi 0.109.0 is breaking the Serve proxy

4 participants

@edoakes @aslonnie @richardliaw @GeneDer