Recently, I had my first data loss due to a Django update since 2012. When Django 4.2 came out, it wanted to migrate the DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings to the new variable but got some weird issues with sorl_thumbnail.
I left it then and forgot about it. When we upgraded to Django 5.1, django-storagesbroke silently by not being able to create a connection to AWS S3. Still, all uploads seemed to have worked but nothing ended up in the buckets. Since it’s very difficult to build, I don’t have any integration tests for my storage.
After fixing the settings variables, the packages worked again but for two days, all uploads were lost.
Why I’m creating this ticket: I’d love to have some kind of hard warning or trigger that I have to do something.
django-upgrade didn’t fix it for me
No system check telling me that I still have outdated / invalid variables lying around in my settings
To be clear: It’s 100% my fault and I don’t blame anyone. But if it can happen to me, it might be happing again. And since Django has a focus on smooth upgrades, I think we should do something about it.
So, the new STORAGES has been around since Django 4.2. The old settings were deprecated then, and have been issuing warnings for the whole time since. The advice on How to upgrade Django shows running with those warnings enabled, but they’ve been noisy by default for a full major version, and you can use the -Werror flag to make that stronger still.
The issue here isn’t so much that the deprecation period expired by that there was no error raised in Django-storages it seems.
We had similar issue in django-compressor (that I have a related thread on) but in that case it simply broke on 5.1 prior to our changes. We had a tracking ticket for the entire time between the 4.2 alpha and the 5.1 alpha, when we finally addressed it.
When updating, I will add an assert to my projects along the lines of assert not DJ51, "Must update to new STORAGES setting before upgrade". I find that quite manageable.
I’m wondering what else Django itself might reasonably do here?
[…] and have been issuing warnings for the whole time since
Did they? I always update very close to release and I’ve never seen a system check warning or a Python warning (which are active, I’m able to see others).
If you are right, this would be a very stupid thing, no matter the reason. Nontheless, if it was a Python warning, many people don’t know that you can activate them and they are somehow implicit and hidden.
Solutionwise, I’m thinking about maybe a list of deprecated settings which we could check for in a system check. This would help people coming from even older version, too.
Yes, the warning works. I just double-checked with Django 4.2.15:
$ python -Xdev -m IPython
Python 3.12.2 (main, Feb 25 2024, 03:55:42) [Clang 17.0.6 ]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.26.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: from django.conf import settings
In [2]: settings.configure(DEFAULT_FILE_STORAGE="something")
/.../django/conf/__init__.py:343: RemovedInDjango51Warning: The DEFAULT_FILE_STORAGE setting is deprecated. Use STORAGES instead.
warnings.warn(DEFAULT_FILE_STORAGE_DEPRECATED_MSG, RemovedInDjango51Warning)
I would be +1 on this. It could just be a general warning, mapping the setting name to the removed version, like “Setting DEFAULT_FILE_STORAGE was removed in Django 5.0.”.
This would help folks who need to jump many Django versions when reviving old projects. On the other hand, for those following the recommended one-version-at-a-time approach, if they don’t react to the deprecation warnings, it’s not clear that an extra warning would do any good.
Yes. There’s nothing Django can do for storage backends that silently “blackhole” files.
@adamchainz As I said, it’s very likely that I was 100% my fault.
But since @adamchainz and @carltongibson agree on a system check, what would be the next step? I talked about this with @sarahboyce before, so what would you suggest? Create a ticket? Create a PR?
Since I believe that this would be a huge improvement for all those people maintaining projects, I’d volunteer for the task
Given the current conversation and agreements (which I do not object), I think we should re-open and re-purpose the ticket to consider “outdated settings”. I’ll do that next Monday to allow for further feedback in this thread.
Yes, nice @nessita Your triage there is exactly as said here.
I lean towards us doing that little bit more because there are these case where things drop off the end of the deprecation period without folks having acted. We’d be well within right to sort of wash our hands, and sometimes we have to. But if we can do something easy/low-cost, then that seems worth the extra affordance.
This ticket might also be related: #35397 (override_settings for STORAGES loses OPTIONS) – Django
Though it’s too late to make updates to the way this was deprecated.
If this is the same issue, I’m confused why @GitRon encountered an issue on 5.1 rather than during the deprecation (4.2 or 5.0)
Awesome @nessita! Thanks, I hope I’ll manage to provide something solid in the next days.
One question: How do the deployment checks work internally? Do they look for system checks or are these a different kind of warnings? Would it make sense to bring this togehter, meaning if you start your Django server, you’ll get a warning and if you do manage.py check --deploy, you’ll see it, too?
)
— but, yes, that one has timed out