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

fs.url(): expose response content headers for pre-signed URLs #451

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Jan 3, 2024

Exposes the fields for setting Content-Disposition, Content-Type, Content-Encoding, Content-Language response headers on a pre-signed URL. This can be useful when the desired user-facing downloaded filename is not necessarily the same as the blob path name.

Comment on lines +1661 to +1663
def sign(self, path, expiration=100, **kwargs):
"""Create a signed URL representing the given path."""
return self.url(path, expires=expiration, **kwargs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

s3fs uses url() for generating signed URLs, which I'm assuming is why this was originally implemented as url() in adlfs, but the top level fsspec defines actually defines sign() for this purpose:
https:/fsspec/filesystem_spec/blob/ac589d8318d761ab6e198e00ba6cbd6c663732d7/fsspec/spec.py#L1536C5-L1537C59

(s3fs.sign() just calls s3fs.url())

Comment on lines +1494 to +1497
content_disposition=content_disposition,
content_encoding=content_encoding,
content_language=content_language,
content_type=content_type,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In s3fs this is supported by passing **kwargs directly into the aws/boto call, but it seems safer to be more explicit rather than passing kwargs into the generate_blob_sas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gcsfs.sign() also just passes **kwargs directly into the signing call, so I would not be opposed to making that change here if it's preferred

@efiop efiop merged commit 576fb7a into fsspec:main Feb 5, 2024
4 checks passed
@pmrowla pmrowla deleted the sign-url-content branch February 6, 2024 03:38
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