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.

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

⬆️ Upgrade Starlette to >=0.37.2,<0.38.0, remove Starlette filterwarning for internal tests #11266

Merged
merged 4 commits into from Apr 2, 2024

Conversation

nothielf
Copy link
Contributor

@nothielf nothielf commented Mar 10, 2024

Upgrading Starlette should solve

DeprecationWarning: The 'app' shortcut is now deprecated. Use the explicit style 'transport=WSGITransport(app=...)' instead.                                                                                                               

When running tests using TestClient

Also, removed filterwarning for method parameter. That was removed on Starlette 0.35.1

@nothielf nothielf changed the title ⬆️ Upgrade Starlette to >=0.37.0,<0.38.0 ⬆️ Upgrade Starlette to >=0.37.0,<0.38.0, Remove starlette param filterwarning Mar 10, 2024
@flying-sheep
Copy link

should be 0.37.1 because of a bug in starlette 0.37.0 I think.

pyproject.toml Outdated Show resolved Hide resolved
@ar090
Copy link

ar090 commented Mar 18, 2024

Thanks for putting this together! Would love to see this merged as it also fixes the issue discussed here

@nothielf nothielf requested a review from alfreedom March 19, 2024 00:08
@ar090
Copy link

ar090 commented Mar 20, 2024

I noticed theres a few other PR's sprouting up that are attempting to make similar changes:
#11315
#11320
@alfreedom any chance you would be available to ✅ this one? 🙏

@alfreedom
Copy link

I noticed theres a few other PR's sprouting up that are attempting to make similar changes: #11315 #11320 @alfreedom any chance you would be available to ✅ this one? 🙏

Approved, sorry for the delay.

@ar090
Copy link

ar090 commented Mar 21, 2024

thanks for the help here @alfreedom! @nothielf are we good to merge or waiting on anything else? Sorry to keep bumping this, we are being forced to upgrade for a vulnerability that comes with deadlines from compliance.

@nothielf
Copy link
Contributor Author

i dont have write access to merge. but it seems is good to go :)

@nothielf
Copy link
Contributor Author

@alfreedom are you able to merge it?

@alfreedom
Copy link

@alfreedom are you able to merge it?

Nope, just maintainers have write access, maybe @alejsdev

@@ -41,7 +41,7 @@ classifiers = [
"Topic :: Internet :: WWW/HTTP",
]
dependencies = [
"starlette>=0.36.3,<0.37.0",
"starlette>=0.37.2,<0.38.0",
Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

This should be good?

Suggested change
"starlette>=0.37.2,<0.38.0",
"starlette>=0.36.3,<0.38.0",

Copy link

@tvasenin tvasenin Mar 25, 2024

Choose a reason for hiding this comment

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

No, minimal version should NOT be less than 0.37.2, as it fixes DeprecationWarning described in the first post.
Also, 0.37.1 fixes another, very important regression: see #11320.

@nothielf nothielf changed the title ⬆️ Upgrade Starlette to >=0.37.0,<0.38.0, Remove starlette param filterwarning ⬆️ Upgrade Starlette to >=0.37.2,<0.38.0, Remove starlette param filterwarning Mar 25, 2024
@ar090
Copy link

ar090 commented Mar 28, 2024

@Kludex sorry to bug you on this but is this something you can merge by chance? Not sure who has the ability to merge for FastAPI

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Mar 28, 2024

Only @tiangolo .

@andgoldin
Copy link

I would like to see this get merged soon as well. Like others, I'm trying to update FastAPI to resolve CVE-2024-24762, but am running into the starlette issue discussed in #11019 when trying to do so.

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Mar 28, 2024

You can just bump Python-multipart.

@estebanx64 estebanx64 added dependencies Pull requests that update a dependency file awaiting-review labels Apr 1, 2024
@aswanidutt87
Copy link

Only @tiangolo .

@tiangolo , please merge this change as we are stuck with a CVE and are keep skipping it from sometime now.
Thank you.

@tiangolo tiangolo changed the title ⬆️ Upgrade Starlette to >=0.37.2,<0.38.0, Remove starlette param filterwarning ⬆️ Upgrade Starlette to >=0.37.2,<0.38.0, remove Starlette filterwarning for internal tests Apr 2, 2024
@tiangolo tiangolo merged commit 0cf5ad8 into tiangolo:master Apr 2, 2024
25 checks passed
@tiangolo
Copy link
Owner

tiangolo commented Apr 2, 2024

Great, thanks for the contribution @nothielf! 🚀

And thanks all for the comments. ☕

This will be available in FastAPI 0.110.1, released in the next hours. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet