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

Handle Content-Type and Content-Disposition headers #90

Merged
merged 8 commits into from
May 14, 2021

Conversation

amercader
Copy link
Contributor

This follows up discussion on #88

Added new `get_mime_type()` method to `StreamingStorage` so that
different backends can implement it. For local storage we use the
builitin `mimetypes` module to guess the mime type from the filename,
for Azure we get it from the blob properties.

In the basic streaming transfer handler we read the value of this method
and add it to the `Content-type` header.
This is currently inferred from the filename using `mimetypes`
giftless/storage/__init__.py Outdated Show resolved Hide resolved
giftless/storage/azure.py Outdated Show resolved Hide resolved
giftless/storage/local_storage.py Show resolved Hide resolved
giftless/transfer/basic_streaming.py Show resolved Hide resolved
amercader added 3 commits May 5, 2021 13:19
Also return a default value of `application/octet-stream` on
the`get_mime_type()` method
As we don't have the actual file name that the mimetype module can use
@amercader amercader marked this pull request as ready for review May 5, 2021 13:05
Sometimes when requesting a file from Giftless we don't want it to
receive it as an attachment, eg when loading it in a browser JS widget
or embedding it. This change adds support for a `x-disposition` extra
which storage backends can use to set the appropiate headers when
returning a download URL.
@amercader amercader changed the title Handle Content-Type header Handle Content-Type and Content-Disposition headers May 13, 2021
Copy link
Contributor

@shevron shevron left a comment

Choose a reason for hiding this comment

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

lgtm

amercader added a commit to datopian/ckanext-blob-storage that referenced this pull request May 14, 2021
Actually all these changes do is supporting an incoming `?preview=1` query
argument on download URLs and forward the `x-disposition = inline` extra to
Giftless when requesting the download URL.

At that point datopian/giftless#90 will kick in
and the storage backend will return the appropiate headers.

The only way to pass this parameter all the way down to the action that
requests the downloadURL was to add a new argument to the download
handlers. This would break backwards compatibility so I've added code to
support plugin hooks that don't support the new `inline` arg.
@amercader
Copy link
Contributor Author

Thanks @shevron I missed your comment regarding the mime type guessing utility function and pushed 9953832 to address it.

Are you able to do a new release including these changes or should I chase someone else at Datopian to do it?

@shevron shevron merged commit 06f0f2c into datopian:master May 14, 2021
@shevron
Copy link
Contributor

shevron commented May 14, 2021

@amercader I've just issued a new tag / release, Docker image with version 0.5.0 should be available soon. BTW, can you take a look at #80 and if you think this release addresses that issue, close it?

Thanks for the PR :)

@amercader amercader deleted the feature/content-type branch May 17, 2021 09:53
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.

2 participants