jschneier / django-storages

https://django-storages.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
2.74k stars 859 forks source link

[s3] - Error when updating to v.1.14.3 client_config #1410

Closed mgzwarrior closed 3 months ago

mgzwarrior commented 3 months ago

https://github.com/jschneier/django-storages/pull/1386

Here is the error that we received:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/django/core/files/storage/handler.py", line 35, in __getitem__
    return self._storages[alias]
KeyError: 'reports'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/src/app/./manage.py", line 22, in <module>
    main()
  File "/usr/src/app/./manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 416, in execute
    django.setup()
  File "/usr/local/lib/python3.9/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/local/lib/python3.9/site-packages/django/apps/registry.py", line 116, in populate
    app_config.import_models()
  File "/usr/local/lib/python3.9/site-packages/django/apps/config.py", line 269, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/local/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/usr/local/lib/python3.9/site-packages/ddtrace/internal/module.py", line 220, in _exec_module
    self.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/usr/src/app/reports/models.py", line 24, in <module>
    class Report(TimeStampedModel):
  File "/usr/src/app/reports/models.py", line 28, in Report
    storage=storages['reports'])
  File "/usr/local/lib/python3.9/site-packages/django/core/files/storage/handler.py", line 43, in __getitem__
    storage = self.create_storage(params)
  File "/usr/local/lib/python3.9/site-packages/django/core/files/storage/handler.py", line 55, in create_storage
    return storage_cls(**options)
  File "/usr/local/lib/python3.9/site-packages/storages/backends/s3.py", line 330, in __init__
    if self.client_config is None:
AttributeError: 'S3ReportsFileStorage' object has no attribute 'client_config'

See here - https://github.com/jschneier/django-storages/pull/1386/files#r1640348739

Note that we are currently using S3Boto3Storage which is deprecated. We have a custom storage like this that provides default settings, plus a number of custom settings via env variables:

from storages.backends.s3boto3 import S3Boto3Storage
from storages.utils import setting

class CustomS3Boto3Storage(S3Boto3Storage):
    // default settings via env vars

class S3StaticFileStorage(CustomS3Boto3Storage):
    setting_prefix = 'STATICFILES_STORAGE'

class S3ReportsFileStorage(CustomS3Boto3Storage):
    setting_prefix = 'REPORTS_STORAGE'
jschneier commented 3 months ago

Thanks for the report.

The deprecated name is just an alias for the base one.

Am a bit confused about how this is possible because the BaseSettings should populate the client_config property. This seems like a thing where the test suite should fail.

Do you have any insight into how you ended up with this error?

mgzwarrior commented 3 months ago

Thanks for the report.

The deprecated name is just an alias for the base one.

Am a bit confused about how this is possible because the BaseSettings should populate the client_config property. This seems like a thing where the test suite should fail.

Do you have any insight into how you ended up with this error?

I don't have any deeper insights because we "solved" this by pinning to version 1.14.2 for now. If I have some free time this week, I will dig a bit deeper and see if I can figure anything out. I would like to clean up the use of S3Boto3Storage in favor of the regular S3Storage since it sounds like they are identical in functionality.

I agree that it feels like a test should fail if this was truly an issue.

jschneier commented 3 months ago

Not an issue please see my comment: https://github.com/jschneier/django-storages/pull/1386#discussion_r1643372905