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

support cloud completely #954

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

tuan-nng
Copy link

@tuan-nng tuan-nng commented Aug 13, 2019

use default-storage to store file instead of using FileSystemStorage

We will not support python 2.7

@tuan-nng tuan-nng force-pushed the support-cloud branch 2 times, most recently from e5531f0 to 3c7d4e3 Compare August 13, 2019 21:00
@karyon
Copy link
Contributor

karyon commented Aug 17, 2019

Can you give any more detail what is broken/suboptimal right now and what this patch improves exactly?

Also, since django 1.11 (which supports python 2.7) is supported until april 2020, i'd rather not break python 2.7 support now already.

@tuan-nng
Copy link
Author

when the cache checks the modified time of a file, it calls os and checks against local file system. This makes this library impossible to use cloud storage as a backend storage completely.

This PR tries to get that check by calling default_storage module.

@tuan-nng
Copy link
Author

Also, since django 1.11 (which supports python 2.7) is supported until april 2020, i'd rather not break python 2.7 support now already.

Yes, I will not merge this PR. Close this for now

@tuan-nng tuan-nng closed this Aug 21, 2019
@karyon
Copy link
Contributor

karyon commented Aug 27, 2019

I'd keep this open, I can wait half a year :)

@karyon karyon reopened this Aug 27, 2019
@karyon
Copy link
Contributor

karyon commented Jan 2, 2020

we've dropped support for python 2.7, so this could be merged.

however, i have tried to understand the code surrounding this and it seems like this only applies if offline compression is not enabled. So if you used a cloud storage for your static assets AND don't use offline compression, doesn't that imply compressor potentially downloads assets from your cloud storage, processes them, and uploads them again all while some client is requesting a page? why would you want to do that? While I'm not going to stop you, I'm trying to understand the use case this change is enabling :)

for the protocol, i checked whether the other .opens in the codebase need this change as well. the only other relevant one was in CompilerFilter.input, and that one handles the output of calls to local executables, which i guess always output local files.

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