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 CORS headers to Azurite #6784

Merged
merged 9 commits into from Apr 17, 2020
Merged

Add CORS headers to Azurite #6784

merged 9 commits into from Apr 17, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 16, 2020

I may be missing something because the URLs generated by Django are not considering STATIC_URL and pointing them to localhost:10000 but they are still pointing to community.dev.readthedocs.io for some reason.

Besides that, I think the rest of the changes are all the required ones.

Closes #6724

@humitos humitos added the PR: work in progress label Mar 16, 2020
@ericholscher
Copy link
Member

@ericholscher ericholscher commented Apr 16, 2020

Is this still needed?

@humitos
Copy link
Member Author

@humitos humitos commented Apr 16, 2020

It's better to have it. Currently we are hitting NGINX and proxy the request to the storage instead of hitting the storage directly --which is the correct thing.

STATIC_URL = f'http://{PRODUCTION_DOMAIN}/devstoreaccount1/static/'
# storage:10000 is running the Azure Blob Storage and it's exposed to the
# host on the same port that user's can access via localhost
STATIC_URL = f'http://localhost:10000/devstoreaccount1/static/'
Copy link
Member

@ericholscher ericholscher Apr 16, 2020

Choose a reason for hiding this comment

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

Should it continue to use PRODUCTION_DOMAIN, but on port 10000? That seems a bit cleaner, and should still resolve to localhost.

Copy link
Member Author

@humitos humitos Apr 17, 2020

Choose a reason for hiding this comment

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

Yeah. I think you are right. I may messed things up here.

I will update this PR with latest master and see if it still works :)

Copy link
Member Author

@humitos humitos Apr 17, 2020

Choose a reason for hiding this comment

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

So, yes. Although, the way to specify this is with our AZURE_STATIC_STORAGE_HOSTNAME that it's used to override the domain returned by storage.url in our Mixin.

This STATIC_URL just need to be /devstorageaccount1/static/

BTW, I'm using assets.community.dev.readthedocs.io so it's a different domain and it follows the same idea than production.

@humitos humitos removed the PR: work in progress label Apr 17, 2020
@humitos humitos marked this pull request as ready for review Apr 17, 2020
@humitos humitos merged commit 8e78d68 into master Apr 17, 2020
2 checks passed
@humitos humitos deleted the humitos/cors-and-init branch Apr 17, 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.

2 participants