jazzband / django-avatar

A Django app for handling user avatars.
BSD 3-Clause "New" or "Revised" License
808 stars 303 forks source link

RemovedInDjango51Warning: The DEFAULT_FILE_STORAGE setting is deprecated #245

Closed christianwgd closed 3 months ago

christianwgd commented 6 months ago

In Django 5 I get the deprecation warning: RemovedInDjango51Warning: The DEFAULT_FILE_STORAGE setting is deprecated. Use STORAGES instead.

Simply replacing STORAGE = settings.DEFAULT_FILE_STORAGE by STORAGE = settings.STORAGES works for me.

This seems to be working from Django 4.2 on. Don't know, which Django version you want to support. Currently the requirements in the setup.py points to 3.2 to 4.2, so maybe there should be some kind of switch for the different Django versions.

PetrDlouhy commented 3 months ago

When I run django-avatar on Django 5.1 I got following error:

Traceback (most recent call last):
  File "/home/petr/soubory/programovani/blenderkit/blenderhub_server/manage.py", line 28, in <module>
    execute_from_command_line(sys.argv)
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 416, in execute
    django.setup()
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/apps/registry.py", line 116, in populate
    app_config.import_models()
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/apps/config.py", line 269, in import_models
    self.models_module = import_module(models_module_name)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/avatar/models.py", line 16, in <module>
    from avatar.conf import settings
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/avatar/conf.py", line 6, in <module>
    class AvatarConf(AppConf):
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/avatar/conf.py", line 27, in AvatarConf
    STORAGE = settings.DEFAULT_FILE_STORAGE
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/petr/.cache/pypoetry/virtualenvs/blenderkit-server-eqrfB9dY-py3.11/lib/python3.11/site-packages/django/conf/__init__.py", line 83, in __getattr__
    val = getattr(_wrapped, name)
          ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Settings' object has no attribute 'DEFAULT_FILE_STORAGE'

@christianwgd I think that the replacement with STORAGE = settings.STORAGES is not a correct fix. The STORAGE settings is not used when Django 4.2 style storages are in use. So it doesn't matter what value this variable has. More correct solution would be probably removing that whole line.

And workaround for Django 5.1 is adding DEFAULT_FILE_STORAGE = "foo" to your settings without need to modify django-avatar code.

christianwgd commented 3 months ago

You may be right, as I stated above: "This seems to be working from Django 4.2 on. Don't know, which Django version you want to support. Currently the requirements in the setup.py points to 3.2 to 4.2, so maybe there should be some kind of switch for the different Django versions."

So please include something like:

from django import VERSION as DJANGO_VERSION

if DJANGO_VERSION >= (5, 0):
    STORAGE = settings.STORAGES
else:
    STORAGE = settings.DEFAULT_FILE_STORAGE
christianwgd commented 3 months ago

Seems like adding (as PetrDlouhy stated above))

DEFAULT_FILE_STORAGE = STORAGES

to your settings.py is at least a workaround for this issue (as long as it is an "issue", maybe one of the authors could tell).

I'm not really sure, if this is intended behaviour. Seems a little bit too much indirections for me. There are two defaults set in config.py. None of them to fixes the default storage for django 5.1.

AVATAR_STORAGE = settings.DEFAULT_FILE_STORAGE
AVATAR_STORAGE_ALIAS = "default"
PetrDlouhy commented 3 months ago

@christianwgd I added a fix in PR #248. Could you please review?

The line

AVATAR_STORAGE = settings.DEFAULT_FILE_STORAGE

is useless with the new style storages which are the only option in Django 5.1.

christianwgd commented 3 months ago

Hey Petr, thanks for the quick response. Give me some time, I will review your changes.

christianwgd commented 3 months ago

Looks good to me. I couldn't check if it's running fine directly since your changes seem not to belong to a changeset in this repo, but made the changes in my repo to check and found that everything runs fine. I did not check for older Django versions.

PetrDlouhy commented 3 months ago

OK, I merged it to main branch, but I don't have rights for a release. @johanneswilm @brosner @ericflo Would you be able to make the new release, please?

atom-tr commented 2 months ago

How long does it take for a new release to be available on the PyPI server? I'm looking for it. 🫡🫡

johanneswilm commented 2 months ago

I'm on it

johanneswilm commented 2 months ago

@atom-tr It has been released. @PetrDlouhy Can you make tags? Once the version number has been updated and a tag placed on that commit, I get a special kind of message from jazzband asking me to review the release.

PetrDlouhy commented 2 months ago

@johanneswilm Thank you very much. I expect thet the tags are meant for next time since now everything seems to be all right. I will do it next time.

johanneswilm commented 2 months ago

@PetrDlouhy Yes, just FYI for next time. I don't actually know if you have access rights to add tags, but if you do, please add them. Then I receive an email directly from jazzband. I receive hundreds of emails from github daily, so it's only luck if I happen to read a github notification.