[Django] #22351: New django migrations and places where lambdas are supposed
Django
-------------------------------+--------------------------------
Reporter: dimyur27@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords: migrations lambdas
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------
In Django's model fields there are a lot of places where lambdas are the
most obvious way to get things done.
Many keyword arguments can accept callable objects which do not supposed
to do complex logic, but some simple things.
E.g. ForeignKey.limit_choices_to, FileField.upload_to...
Example from Django Docs:
limit_choices_to = lambda: {'pub_date__lte': datetime.date.utcnow()}
But Django's migrations cannot cope with lambdas passed as keyword
arguments, so that fact either should be mentioned in the docs (let
examples use named functions rather than lambdas) or community has to put
extra attention on migrations+lambdas problem.
Thanks for the magnificent framework! My best regards.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Agreed this should at least be mentioned in docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:1>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by chriscauley):
This is a similar problem to tickets #22373 and #22436 and loosely related
to a few others. In #22436 we discovered that if we do
`limit_choices_to=module.some_function`, migrations will break in the
event that module.some_function is deleted, changed, renamed, or moved.
This makes migrations not future safe. I'm pretty new to django
migrations, but I know with south the overall goal was to have previous
migrations not rely on the contemporary state of models.py for this very
reason.
Is the `limit_choices_to` function even used in migrations? If we make the
`Field.deconstruct()` method return a function that raises a
NotImplementedError when called then it wouldn't mind the lambda. It would
also be future safe in the event that the function provided is renamed or
changed in a way that no longer reflects the code. I'll make a patch to
reflect this change.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:2>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by chriscauley):
I tried to reproduce this specific issue and I don't get any error when
running `python manage.py makemigrations APP_NAME` with a lambda in
limit_choices_to. That kwarg is not returned in
`ForeignKey.deconstruct()`, so the migration inspector doesn't care if it
is a lambda. Can you show me code to illustrate what you mean?
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:3>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dimyur27@…):
When instead of get_user_picture_path there was a lambda, it refused to do
"makemigrations".
{{{
class MyModel(models.Model):
picture = models.ImageField(
upload_to=get_user_picture_path,
blank=True,
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:4>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by chriscauley):
I thought you specifically were having a problem with `limit_choices_to`.
That makes more sense. `upload_to` is addressed in #22436 where
`SomeClass.some_method` breaks in the same way. The solution there will
fix lambdas as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:5>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dimyur27@…):
I just wanted the docs to mention about these migration nuances when
describing such kwargs of the fields. To prevent the newbies (as I am)
getting into trouble.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:6>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by stefankoegl):
* cc: stefankoegl (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:7>
Django
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by niwi):
* cc: niwi@… (added)
Comment:
It also happens with simple charfield and "default" keyword argument like
this:
{{{#!python
token = models.CharField(max_length=255, blank=True, unique=True,
db_index=True,
default=lambda: str(uuid.uuid1()))
}}}
This is a traceback:
{{{
Traceback (most recent call last):
File "manage.py", line 8, in <module>
execute_from_command_line(sys.argv)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/__init__.py", line
385, in execute_from_command_line
utility.execute()
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/__init__.py", line
377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/base.py", line 288,
in run_from_argv
self.execute(*args, **options.__dict__)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/base.py", line 337,
in execute
output = self.handle(*args, **options)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/commands/makemigrations.py",
line 115, in handle
self.write_migration_files(changes)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/core/management/commands/makemigrations.py",
line 143, in write_migration_files
migration_string = writer.as_string()
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 124,
in as_string
operation_string, operation_imports =
OperationWriter(operation).serialize()
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 76,
in serialize
arg_string, arg_imports = MigrationWriter.serialize(item)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 229,
in serialize
item_string, item_imports = cls.serialize(item)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 290,
in serialize
return cls.serialize_deconstructed(path, args, kwargs)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 205,
in serialize_deconstructed
arg_string, arg_imports = cls.serialize(arg)
File "/home/niwi/.virtualenvs/table/lib/python3.4/site-
packages/Django-1.7b4-py3.4.egg/django/db/migrations/writer.py", line 303,
in serialize
raise ValueError("Cannot serialize function: lambda")
ValueError: Cannot serialize function: lambda
}}}
Django version: 0cabf3a (2014-06-21) revision from git repository.
Without default keyword with lambda, everything works ok.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:8>
Django
with migrations)
--------------------------------------+------------------------------------
Reporter: dimyur27@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by timo):
* component: Migrations => Documentation
* easy: 0 => 1
Comment:
The fact that Django cannot serialize lambdas was added in [6bbb8200].
There are a couple places where we document the use of a lambda which
should be cleaned up:
{{{
ref/models/fields.txt:a dictionary as the default, use a ``lambda`` as
follows::
ref/models/fields.txt: contact_info = JSONField("ContactInfo",
default=lambda:{"email": "t...@example.com"})
ref/models/fields.txt: limit_choices_to = lambda: {'pub_date__lte':
datetime.date.utcnow()}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:9>
Django
with migrations)
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by timo):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:10>
Django
with migrations)
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: migrations lambdas | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by claudep):
You have a backtick left after "function". That aside, I think that adding
a quick note about lambdas being problematic with migrations wouldn't hurt
(with a link to `migration-serializing`), at least in the `default`
section.
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:11>
Django
with migrations)
Reporter: dimyur27@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: migrations lambdas | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:12>
Django
with migrations)
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: migrations lambdas | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"5ebf03b7dd2d1c215f5c0b725083d36379a7ac5b"]:
{{{
#!CommitTicketReference repository=""
revision="5ebf03b7dd2d1c215f5c0b725083d36379a7ac5b"
Fixed #22351 -- Removed usage of lambdas in model field options.
Thanks claudep for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:13>
Django
with migrations)
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: migrations lambdas | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"9673013abe4cd64183bf55d89adfa04927e3b947"]:
{{{
#!CommitTicketReference repository=""
revision="9673013abe4cd64183bf55d89adfa04927e3b947"
[1.7.x] Fixed #22351 -- Removed usage of lambdas in model field options.
Thanks claudep for review.
Backport of 5ebf03b7dd from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22351#comment:14>