Skip to content

Deprecate FileResponse(method=...) parameter#2366

Merged
Kludex merged 4 commits intomasterfrom
deprecate-method-parameter
Dec 16, 2023
Merged

Deprecate FileResponse(method=...) parameter#2366
Kludex merged 4 commits intomasterfrom
deprecate-method-parameter

Conversation

@Kludex
Copy link
Owner

@Kludex Kludex commented Dec 10, 2023

The server drops the body on HEAD anyway, so even if we don't have this logic at all, the client still receives an empty body.

The idea of still having to check the method is to avoid opening the file.

@Kludex Kludex requested a review from a team December 10, 2023 08:08
self.send_header_only = method is not None and method.upper() == "HEAD"
if method is not None:
warnings.warn(
"The 'method' parameter is not used, and it will be removed.",
Copy link
Owner Author

@Kludex Kludex Dec 10, 2023

Choose a reason for hiding this comment

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

The method parameter becomes a no-op with this PR, and it's "scheduled" to be removed in 1.0.

file.write(content)

app = FileResponse(path=path, filename="example.png")
client = test_client_factory(app)
Copy link
Owner Author

Choose a reason for hiding this comment

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

This test is actually useless, since the TestClient drops the body on HEAD methods.

I'll create a new test later, but if you want to approve, it will speed up the process here. 👀

@Kludex Kludex mentioned this pull request Dec 10, 2023
3 tasks
@Kludex Kludex force-pushed the deprecate-method-parameter branch from 32a0c4d to 3e82824 Compare December 16, 2023 12:36
@Kludex Kludex enabled auto-merge (squash) December 16, 2023 12:39
@Kludex Kludex merged commit 6715eb4 into master Dec 16, 2023
@Kludex Kludex deleted the deprecate-method-parameter branch December 16, 2023 12:47
mvdbeek added a commit to galaxybot/galaxy that referenced this pull request Jan 30, 2024
The ``send_header_only`` attribute has been removed in starlette
(Kludex/starlette#2366), and replaced with
`scope["method"].upper() == "HEAD"`
mvdbeek added a commit to galaxybot/galaxy that referenced this pull request Jan 30, 2024
The ``send_header_only`` attribute has been removed in starlette
(Kludex/starlette#2366), and replaced with
`scope["method"].upper() == "HEAD"`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants