[Django] #34192: Callable storage on FileField fails to deconstruct when it returns default_storage
Django
default_storage
-----------------------------------------+------------------------
Reporter: Matt Westcott | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
If the `storage` argument on a FileField is set to a callable that returns
`default_storage`, it is omitted from the deconstructed form of the field,
rather than being included as a reference to the callable as expected.
For example, given a model definition:
{{{
from django.core.files.storage import FileSystemStorage, default_storage
from django.db import models
import random
other_storage = FileSystemStorage(location='/media/other')
def get_storage():
return random.choice([default_storage, other_storage])
class MyModel(models.Model):
my_file = models.FileField(storage=get_storage)
}}}
repeatedly running `makemigrations` will randomly generate a migration
that alternately includes or omits `storage=myapp.models.get_storage` on
the FileField definition.
This case was overlooked in the fix for #31941 - the deconstruct method
tests `if self.storage is not default_storage` to determine whether to add
the `storage` kwarg, but at this point `self.storage` is the evaluated
version, so it wrongly returns false for a callable that returns
`default_storage`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34192>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
default_storage
Reporter: Matt Westcott | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
Yes, OK. That looks correct. We're essentially saying that when
`hasattr(self, "_storage_callable")` that should unconditionally be used
by `deconstruct`. 🤔
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:1>
Django
default_storage
Reporter: Matt Westcott | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* component: Migrations => Database layer (models, ORM)
Comment:
We should probably use `getattr(self, "_storage_callable", self.storage)`
in both lines.
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:2>
Django
default_storage
-------------------------------------+-------------------------------------
| Panico
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Francesco Panico
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:3>
Django
default_storage
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Francesco Panico => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:4>
Django
default_storage
-------------------------------------+-------------------------------------
| Yadav
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Abhinav Yadav):
* owner: (none) => Abhinav Yadav
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:5>
Django
default_storage
-------------------------------------+-------------------------------------
| Westcott
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Abhinav Yadav => Matt Westcott
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:6>
Django
default_storage
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Matt
| Westcott
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Matt Westcott):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/16493
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:7>
Django
default_storage
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Matt
| Westcott
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:8>
Django
default_storage
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Matt
| Westcott
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ef85b6bf0bc5a8b194f0724cf5bbedbcee402b96" ef85b6b]:
{{{
#!CommitTicketReference repository=""
revision="ef85b6bf0bc5a8b194f0724cf5bbedbcee402b96"
Fixed #34192 -- Preserved callable storage when it returns
default_storage.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:9>
Django
default_storage
-------------------------------------+-------------------------------------
Reporter: Matt Westcott | Owner: Matt
| Westcott
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b332a96cd721defb6ff13ba1bc57958a984f0773" b332a96]:
{{{
#!CommitTicketReference repository=""
revision="b332a96cd721defb6ff13ba1bc57958a984f0773"
[4.2.x] Fixed #34192 -- Preserved callable storage when it returns
default_storage.
Backport of ef85b6bf0bc5a8b194f0724cf5bbedbcee402b96 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34192#comment:10>