appsembler / edx-platform

Appsembler Tahoe fork of Open edX. Branches: `main` for Juniper and `hawthorn/main` for Hawthorn.
https://appsembler.com/products/tahoe/
GNU Affero General Public License v3.0
13 stars 15 forks source link

Tech Debt: USE_S3_FOR_CUSTOMER_THEMES is hardcoded and should be configurable #341

Closed OmarIthawi closed 4 years ago

OmarIthawi commented 5 years ago

USE_S3_FOR_CUSTOMER_THEMES is somewhat a very bad method of choosing a storage class.

In devstack it is mixed with static files, while it should be a media file.

The proper solution for doing such operation is to use a configurable storage class:

# production
CUSTOMER_THEMES_STORAGE_CLASS = 'storages.backends.s3boto.S3BotoStorage'
CUSTOMER_THEMES_STORAGE_PARAMS = {
  'location': 'customer_themes',
}

# dev
CUSTOMER_THEMES_STORAGE_CLASS = 'django.core.files.storage.FileSystemStorage'
CUSTOMER_THEMES_STORAGE_PARAMS = {}

This way it's fully configurable and the code won't care where it's running.

Also, this needs a cash busting mechanism. So perhaps at save, the css file should be suffixed with some GUID:

appsembler-the-red.staging-tahoe-hawthorn.appsembler.com.e044f8e7-e4bd-44b2-9c46-6af817b1ebbd.css

Deleting all appsembler-the-red.staging-tahoe-hawthorn.appsembler.com.* files before that wouldn't be a bad idea.

abeals commented 5 years ago

I know @OmarIthawi also tagged you on his PR, but @grozdanowski making sure you also saw this related issue to share the full rationale behind the change.

OmarIthawi commented 5 years ago

Thanks @abeals for having :eyes: here, I this this is mostly early Tahoe work in the past that was done quickly. Like any other good Tech Debt, we like the result, but we need a better implementation since now we know a bit better.