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 django-storages to support URLs with more http methods #6771

Merged
merged 4 commits into from Mar 11, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 11, 2020

This PR upgrades django-storages to a not yet released version to support HEAD method when generating storage URLs that are private (include signed tokens).

I checked the changelog and there is nothing breaking as far as I can tell. All the tests that I did locally using Azurite worked properly.

Note that I'm removing azure-storage-blob and azure-storage-common from our pinned versions because those are correctly specified on django-storage dependencies as azure-storage-blob >=1.3.1,<12.0.0.

I'm interesting on merging this sooner than later, so we can have a week or similar of testing locally before deploying it. Best way to test this is to pull down the PR, re-generate the docker image completely (without cache: inv docker.build) and do some storage related operations (build, serve docs, wipe, collectstatic, etc)

@humitos humitos requested review from ericholscher and Mar 11, 2020
Copy link
Member

@ericholscher ericholscher left a comment

Definitely want to get this tested a bit before shipping, so 👍 on merging soon and testing before next deploy.

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Mar 11, 2020

Seems tests aren't installing the proper version.

@humitos
Copy link
Member Author

@humitos humitos commented Mar 11, 2020

It's installing the proper version. However, I didn't realize that this is not going to work as is because I added the support for http_method only in S3 backend and we are using Azure here.

Actually, it does not make sense in Azure because it seems the token is not based on HTTP method 😕

@humitos humitos merged commit 56d1401 into master Mar 11, 2020
3 checks passed
@humitos humitos deleted the humitos/storages-head branch Mar 11, 2020
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.

None yet

2 participants