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

Add content disposition type parameter to FileResponse #1266

Merged
merged 2 commits into from Feb 14, 2022

Conversation

qu1ck
Copy link
Contributor

@qu1ck qu1ck commented Aug 14, 2021

disposition "attachment" causes browsers to download
the file. E.g. "inline" will will be attempted to be
displayed directly.

@qu1ck
Copy link
Contributor Author

qu1ck commented Sep 16, 2021

Friendly ping.
I have a larger PR pending for support of 206 partial content responses that I want to send after this is merged.

Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Documentation needs to be updated on this PR as well.

@@ -243,6 +243,7 @@ def __init__(
filename: str = None,
stat_result: os.stat_result = None,
method: str = None,
content_disposition_type: str = "attachment",
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Only "inline" and "attachment" are allowed here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

@Kludex
Copy link
Sponsor Member

Kludex commented Sep 16, 2021

disposition "attachment" causes browsers to download
the file. E.g. "inline" will will be attempted to be
displayed directly.
@qu1ck
Copy link
Contributor Author

qu1ck commented Sep 17, 2021

I added the parameter description to the docs. There are a couple other parameters missing there but it's outside of the scope of the pull request, just FYI.

@adriangb adriangb added the feature New feature or request label Feb 2, 2022
Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

I'm pretty okay with this yup.
It's a nice low-footprint change, so no big objections.

@tomchristie tomchristie merged commit 6ad5a6a into encode:master Feb 14, 2022
@qu1ck qu1ck deleted the disposition branch February 15, 2022 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants