search

klis87 / django-cloudinary-storage

Django package that provides Cloudinary storages for both media and static files as well as management commands for removing unnecessary files.
MIT License
130 stars 26 forks source link

BASE_DIR setting is assumed #9

Closed yuvadm closed 7 years ago

yuvadm commented 7 years ago

In https://github.com/klis87/django-cloudinary-storage/blob/master/cloudinary_storage/app_settings.py#L45 it is assumed that settings.BASE_DIR exists, and while is is generated in the default settings, not all Django projects choose to use it.

This package should not rely on this settings to exist.

klis87 commented 7 years ago

@yuvadm Good catch, thanks for raising this issue.

So what do you think the best approach should be, putting

BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

inside https://github.com/klis87/django-cloudinary-storage/blob/master/cloudinary_storage/app_settings.py ?

yuvadm commented 7 years ago

@klis87 good question, that's one way to do it. Another option is to take settings.STATIC_ROOT which is known to exist (it's required) and then use ../manifest/ as the path. Probably a better option would be to see how this is implemented in other package and use similar behavior.

Unfortunately I've never gone into the internals of the various storages / caching packages so it would be hard for me to recommend. But yeah, a quick fix would be to do what you suggested.

As for me, I've returned BASE_DIR as a workaround in the meantime.

Thanks for this package by the way, it's super helpful :)

klis87 commented 7 years ago

I am glad you find this package useful :) I uploaded new version to pypi (0.2.2) fixing this issue.