[Django] #22436: migrations fail on custom upload

Django

unread,

Apr 14, 2014, 11:57:26 AM4/14/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField
--------------------------------------------+------------------------
Reporter: David Binetti <dbinetti@…> | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------------+------------------------
One of my models has an ImageField with a dynamically created upload_to:

{{{
class Photo(models.Model):

account = models.ForeignKey(Account)

...

def upload_thumb(instance, filename):
return 'photos/{0}/dropbox{1}'.format(instance.account.id,
filename)

thumbnail = models.ImageField(upload_to=upload_thumb, null=True)
}}}

When I run makemigrations it produces the following (without error):

{{{
# encoding: utf8
from django.db import models, migrations
import apps.dinadesa.models

class Migration(migrations.Migration):

dependencies = [
('dinadesa', '0006_auto_20140414_0836'),
]

operations = [
migrations.AlterField(
model_name='photo',
name='thumbnail',
field=models.ImageField(null=True,
upload_to=apps.dinadesa.models.upload_thumb),
),
]
}}}

(I keep my apps in an `apps` directory in the project root, in case that
matters.)

When I try to run the migration, I get:

{{{
(dinadesa)$ django-admin migrate
Traceback (most recent call last):
File "/Users/dbinetti/.virtualenvs/dinadesa/bin/django-admin", line 11,
in <module>
sys.exit(execute_from_command_line())
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/core/management/__init__.py", line 427, in
execute_from_command_line
utility.execute()
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/core/management/__init__.py", line 419, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/core/management/base.py", line 337, in execute
output = self.handle(*args, **options)
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/core/management/commands/migrate.py", line 62, in handle
executor = MigrationExecutor(connection,
self.migration_progress_callback)
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/db/migrations/executor.py", line 14, in __init__
self.loader = MigrationLoader(self.connection)
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/db/migrations/loader.py", line 48, in __init__
self.build_graph()
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/db/migrations/loader.py", line 145, in build_graph
self.load_disk()
File "/Users/dbinetti/.virtualenvs/dinadesa/lib/python2.7/site-
packages/django/db/migrations/loader.py", line 103, in load_disk
migration_module = import_module("%s.%s" % (module_name,
migration_name))
File
"/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/importlib/__init__.py",
line 37, in import_module
__import__(name)
File
"/Users/dbinetti/Repos/dinadesa/project/apps/dinadesa/migrations/0007_auto_20140414_0850.py",
line 6, in <module>
class Migration(migrations.Migration):
File
"/Users/dbinetti/Repos/dinadesa/project/apps/dinadesa/migrations/0007_auto_20140414_0850.py",
line 16, in Migration
field=models.ImageField(null=True,
upload_to=apps.dinadesa.models.upload_thumb),
AttributeError: 'module' object has no attribute 'upload_thumb'
}}}

Which I can work around by changing the `field` line in my migration to:

{{{
field=models.ImageField(null=True,
upload_to=apps.dinadesa.models.Photo.upload_thumb), # Note the addition
of the `Photo` model in the path.
}}}

Then, the `migrate` command works fine.

BUT if I then try to run another migration it detects the altered field,
and I have to do it all again.

I hope this is sufficient information. It is my first bug report and I'm
not certain how to create a test case, but it is 100% reproduce-able on my
system.

Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Apr 14, 2014, 3:08:38 PM4/14/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by charettes):

* status: new => assigned
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => charettes
* needs_docs: => 0
* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:1>

Django

unread,

Apr 14, 2014, 3:19:41 PM4/14/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

-------------------------------------+-------------------------------------
Reporter: David Binetti | Owner:

* owner: charettes => chriscauley

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:2>

Django

unread,

Apr 14, 2014, 5:07:37 PM4/14/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by chriscauley):

Your specific problem can be easily fixed by moving the upload_thumb
method outside of the Photo class. Unless call Photo.upload_thumb anywhere
there's no difference between the two. There's nothing wrong with how you
did it before, but there's no way to implement migrations with as you had
it (except manually typing qualname for Photo.upload_to like you did).
I'll expand on this in a second comment, but for now here's your solution.

{{{

def upload_thumb(instance, filename):
return 'photos/{0}/dropbox{1}'.format(instance.account.id, filename)

class Photo(models.Model):

account = models.ForeignKey(Account)

...

thumbnail = models.ImageField(upload_to=upload_thumb, null=True)
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:3>

Django

unread,

Apr 14, 2014, 6:03:49 PM4/14/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by chriscauley):

The migration sterilizer previously was using the `__name__` of the
function and the module, which is not the full import path if the function
is a method of a class.

{{{
...
else:
module = value.__module__
return"%s.%s" % (module, six.get_qualname(value)),
set(["import %s" % module])
# formerly
# return"%s.%s" % (module, value.__name__), set(["import
%s" % module])
}}}

`six.get_qualname(func)` is a new function that returns
`func.__qualname__` for python 3.X and fakes it for 2.X (sort of). It
relies on `func.im_class`, which isn't always available in python 2.X:

{{{
def upload1(instance,self):

return 'photos/{0}/dropbox{1}'.format(instance.account.id, filename)

class NotAModel:
def upload2(instance,self):

return 'photos/{0}/dropbox{1}'.format(instance.account.id,
filename)

class Subclass:
def upload3(instance,self):

return 'photos/{0}/dropbox{1}'.format(instance.account.id,
filename)

class MyModel(models.Model):
def upload4(instance,self):

return 'photos/{0}/dropbox{1}'.format(instance.account.id,
filename)

photo_1 = models.ImageField(upload_to=upload1)
photo_2 = models.ImageField(upload_to=NotAModel.upload2)
photo_3 = models.ImageField(upload_to=NotAModel.Subclass.upload3)
photo_4 = models.ImageField(upload_to=upload4)
}}}

Here `photo_1` and `photo_2` will work in Python 2 and 3 (with the
modifications on the branch mentioned below). `photo_3` can't be
serialized in 2.X because there's no way to access `NotAModel` from
`Subclass`. `photo_4` can't be serialized because `upload_to` is passed in
the unbound function `upload4` which doesn't have the attribute
`im_class`.

But this all kind of unnecessary because migrations don't actually need
`upload_to`. Additionally, if you rename `upload_to` it will cause past
migrations to break. The best way out I can see is to set the
`upload_to="IN_MIGRATIONS"` in the migrations file and modify the
FileField to ignore upload_to when in migrations. If anyone is interested,
here is a link to the changes listed above and `six.get_qualname`:

https://github.com/chriscauley/django/commit/2a6f27451f9cfe2f76f92a8806b2939dc46ce2a5

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:4>

Django

unread,

Apr 15, 2014, 2:08:01 AM4/15/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by David Binetti <dbinetti@…>):

Thank you for responding. I did move my function outside of the class,
and it works (although it did break migrations as you indicated it would.)
I'm still in dev so no harm, no foul.

As an aside, I am using upload_to to create a dynamically generated path
which is respected by django-storages (for S3, in my case.) Other
attempts to create a path from the object itself failed (for reasons I
can't remember -- but a properly configured upload_to was the solution.)

Thanks again.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:5>

Django

unread,

Apr 15, 2014, 10:24:54 AM4/15/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by chriscauley):

It sounds like you fixed your issue, but here's the solution for anyone
who has this problem before it is patched.

The `upload_to` field should not be used in the migrations at all. If you
go in and change the `upload_to=path.to.some.function` to a string in the
migrations they will work again. The important thing is that the upload to
in the last migration matches the current state since then inspector will
(wrongly) think a change in `upload_to` is a change in the database. You
can also do `python manage.py squashmigrations APPNAME` to nuke old broken
migrations.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:6>

Django

unread,

Apr 15, 2014, 1:29:58 PM4/15/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Has patch: 1 | Triage Stage: Accepted
Needs tests: 1 | Needs documentation: 0

Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by chriscauley):

* has_patch: 0 => 1
* needs_tests: 0 => 1

Comment:

https://github.com/chriscauley/django/commit/5f1707eee60d9a742cfd7f86fb81bebdfca9b01c

That's my proposed solution. The only place where `Field.deconstruct()` is
used is in migrations. Since schema migrations don't use `upload_to ` it's
best to replace it with a dummy function. This is future safe (in the
event that the field's upload_to is altered) and doesn't have the
__qualname__ issues that Python 2.X introduces.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:7>

Django

unread,

May 16, 2014, 10:44:20 AM5/16/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

-------------------------------------+-------------------------------------
Reporter: David Binetti | Owner:
<dbinetti@…> | chriscauley
Type: Bug | Status: assigned

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by MarkusH):

* cc: info@… (added)
* version: 1.7-beta-1 => master
* severity: Normal => Release blocker

Comment:

Django's migrations should represent the ''exact'' state the model has at
a given time. This includes all arguments defined on fields. I therefore
disagree with your change. I'd stick with defining a function outside the
class.

I furthermore set this as a release blocker, as @andrewgodwin should have
a look at this as well.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:8>

Django

unread,

May 16, 2014, 12:04:15 PM5/16/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by chriscauley):

Thank you for upping the severity. I don't understand why it should
"represent the exact state the model has at a given time", and I can think
of strong arguments as to why it shouldn't. Using django==1.3 and south I
can create a model that uses a URLField. If I change the value of the
verify_exists keyword argument, that does not require me to create a new
migration file. If I go up to django==1.4 then suddenly the verify exists
keyword argument is removed and I have to go through and delete it from
every instance that uses it. No biggy though, just one line here and
there. However, I don't have to go back and change my migrations because
verify_exists has no impact on the database and therefore doesn't trigger
a migration.

Now imagine that migrations were built into django back in 1.3 and that
they were made to "represent the exact state the model has at a given
time". I create a URLField, make a migration. I decide I don't like
verify_exists and set it to False, make another migration (not a huge
deal, but pointless). Then I update to 1.4 and so I go through and remove
verify_exists. A migration is now '''impossible''' because
`models.URLField(verify_exists=False)` (which is the previous state of the
model) is not valid django any more (and raises a TypeError as of
django==1.5). In order to make my migrations work again I need to go back
and remove every reference to verify_exists from every migration file. The
migration where I went from verify_exists=True to False now does nothing!
Then again the migration did nothing in the first place because
verify_exists doesn't change the database.

One of my django projects (an online news magazine) has 425 migrations.
`grep URLField * -r|grep migrations/0| wc -l` shows that this represents
6439 lines of code for the 50 URLFields in this project. That's insanity.
That project might still be on 1.3 if upgrading it to 1.4 required
modifying 6.5k lines of code.

This is just one example but I can come up with plenty other like it. In
every case we either have to support legacy code indefinitely (for example
leave verify_exists as a do-nothing key word argument) or else modifying
the Field's API would require going back and editing every existing
migration file. Neither option is pleasant and either way '''none of this
has any effect on the database'''. I think migrations should be minimal
and independent of non-database code.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:9>

Django

unread,

May 16, 2014, 12:11:55 PM5/16/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by MarkusH):

For the why and how to prevent problems with custom fields, I want to
point you to #21499

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:10>

Django

unread,

May 20, 2014, 10:06:47 AM5/20/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by andrewgodwin):

chriscauley: The contract is more precise than that; when fields are used
in a migration you're guaranteeing that they'll be backwards-compatible
for as long as the migration exists. That's easy enough for Django itself,
especially now we have tools to let you reset migrations and remove old
declarations; you'd be able to easy move from your 425 to just a few new
initial ones.

Migrations just being for the database is also not true; as we support
data migrations we need to support all arguments, even those which seem
useless at first (e.g. upload_to, validators). These fields are needed to
have the field truly represented in the migration, otherwise a migration
that added new rows wouldn't be able to do it properly.

Plus, what if a subclass of FileField actually uses upload_to to affect
the database? We can't rule it out for everything.

It's painful, but we've got literally everything else in Django working
well in this regard, pointers to functions on classes is one of the very
few missing things - your suggested use of `six.get_qualname` seems
sensible and I'll investigate it in a bit.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:11>

Django

unread,

May 20, 2014, 11:13:29 AM5/20/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

-------------------------------------+-------------------------------------
Reporter: David Binetti | Owner:

<dbinetti@…> | andrewgodwin

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by andrewgodwin):

* owner: chriscauley => andrewgodwin

Comment:

I can't find this `six.get_qualname` function, and even then, if it uses
`im_func` it's not going to work on function references passed in from
within the class body (as these are not yet methods).

I think this may be impossible to solve on Python 2; we might have to add
a check where the serializer tries to re-import it and fails if it can't
find it, so we can at least give a nice error message.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:12>

Django

unread,

May 22, 2014, 6:49:27 AM5/22/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Althalus):

I'm using 1.7 now and found that migrations fail too my because upload_to
function is generated dynamically.
Consider the folloing example:

{{{
# utils/files.py
def gen_upload_to(fieldname, path, .......):
def upload_to(instance, filename):
pass # some custom logic
return upload_to

# app/models.py
class MyModel(models.Model):
file = models.FileField(upload_to=gen_upload_to('file',
'path/to/files'))

# app/migrations/0001_initial.py
class Migration(migrations.Migration):
operations = [
migrations.CreateModel(
name='MyModel',
fields=[
('file',
models.FileField(upload_to=utils.files.upload_to)),
],
),
]
}}}

Obviously my utils/files.py module doesn't contain `upload_to` function
with results in errors.
I had to create dummy `upload_to` in there to prevent import errors, but
anyway every time I use `makemigrations` command django detect changes in
my `file` field.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:13>

Django

unread,

May 22, 2014, 4:25:35 PM5/22/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

-------------------------------------+-------------------------------------
Reporter: David Binetti | Owner:
<dbinetti@…> | andrewgodwin
Type: Bug | Status: assigned
Component: Migrations | Version:

Severity: Release blocker | 1.7-beta-2

Keywords: | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 1 | Needs documentation: 0

Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):

* version: master => 1.7-beta-2

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:14>

Django

unread,

Jun 4, 2014, 10:22:37 AM6/4/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by semenov):

@Althalus, a very ugly workaround for this would be:

{{{#!python

# utils/files.py
def gen_upload_to(fieldname, path, .......):

import sys
if len(sys.argv) > 1 and sys.argv[1] in ('makemigrations', 'migrate'):
return None # Hide ourselves from Django migrations

def upload_to(instance, filename):
pass # some custom logic
return upload_to
}}}

Too bad Django monitors non-db-related field attributes for no apparent
reason. It's not only upload_to. For instance, it creates a useless
migration even when a field's `help_text` is changed.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:15>

Django

unread,

Jun 5, 2014, 11:44:31 PM6/5/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by andrewgodwin):

You do need the upload_to attribute in migrations, as well as help text,
as they can affect the usage of fields inside RunPython segments, or other
field subclasses might use help_text for something that's not just
display. South tried the whitelist and blacklist approaches here and both
were flawed.

I've consulted with people and there's literally no sane way to get a
pointer to a function like that if it's reused directly from the class
body; the fix is to move the function outside of the class body.

The fix for this will be a check which makes sure the referenced function
is importable and gives the above advice if it is not.

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:16>

Django

unread,

Jun 7, 2014, 8:06:25 PM6/7/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

-------------------------------------+-------------------------------------
Reporter: David Binetti | Owner:
<dbinetti@…> | andrewgodwin

Type: Bug | Status: closed

Component: Migrations | Version:
Severity: Release blocker | 1.7-beta-2

Keywords: | Resolution: fixed

Has patch: 1 | Triage Stage: Accepted
Needs tests: 1 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Andrew Godwin <andrew@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"6fd455adfcc85f6bd390bce784a1b5dfe5d610ea"]:
{{{
#!CommitTicketReference repository=""
revision="6fd455adfcc85f6bd390bce784a1b5dfe5d610ea"
Fixed #22436: More careful checking on method ref'ce serialization
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:17>

Django

unread,

Jun 7, 2014, 8:06:38 PM6/7/14

to django-...@googlegroups.com

#22436: migrations fail on custom upload_to on ImageField

Comment (by Andrew Godwin <andrew@…>):

In [changeset:"98949e3b108100a40cb6ad59be78f33580439fef"]:
{{{
#!CommitTicketReference repository=""
revision="98949e3b108100a40cb6ad59be78f33580439fef"
[1.7.x] Fixed #22436: More careful checking on method ref'ce serialization
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22436#comment:18>

Reply all

Reply to author

Forward

[Django] #22436: migrations fail on custom upload_to on ImageField

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django