#21954 closed Bug (fixed)

1.7a1 - Migration syntax error

Reported by: caulagi Owned by: Andrew Godwin <andrew@…>
Component: Migrations Version: 1.7-alpha-1
Severity: Release blocker Keywords:
Cc: loic@…, caulagi Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by russellm)

I am using the following (simplified) model that doesn't migrate using the new migrations in Django 1.7a1 -

$
$ pip freeze
Django==1.7a1
argparse==1.2.1
distribute==0.6.34
wsgiref==0.1.2
$
$ cat blog/models.py
from django.db import models
from django.utils import timezone

class Blog(models.Model):

    title = models.CharField(max_length=200)
    created = models.DateTimeField(default=timezone.now())

    def __unicode__(self):
        return self.title

$
$ python manage.py makemigrations
Migrations for 'blog':
  0001_initial.py:
    - Create model Blog
$
$ python manage.py migrate
Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/core/management/commands/migrate.py", line 62, in handle
    executor = MigrationExecutor(connection, self.migration_progress_callback)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/db/migrations/executor.py", line 14, in __init__
    self.loader = MigrationLoader(self.connection)
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/db/migrations/loader.py", line 48, in __init__
    self.build_graph()
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/db/migrations/loader.py", line 145, in build_graph
    self.load_disk()
  File "/home/pcaulagi/projects/test/local/lib/python2.7/site-packages/Django-1.7a1-py2.7.egg/django/db/migrations/loader.py", line 103, in load_disk
    migration_module = import_module("%s.%s" % (module_name, migration_name))
  File "/usr/lib/python2.7/importlib/__init__.py", line 37, in import_module
    __import__(name)
  File "/tmp/bar/blog/migrations/0001_initial.py", line 17
    ('created', models.DateTimeField(default=datetime.datetime(2014, 2, 4, 9, 15, 59, 685251, tzinfo=<UTC>))),

                              ^
SyntaxError: invalid syntax
$
$

We have had an initial discussion on django-users - https://groups.google.com/forum/#!topic/django-users/5ryEZd7I1t8

Change History (11)

comment:1 Changed 13 months ago by loic84

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

Hi @caulagi, the problem is that you are not using a callable for the default value of created but an actual datetime object.

Try created = models.DateTimeField(default=timezone.now) instead; notice the missing parentheses after "timezone.now".

comment:2 Changed 13 months ago by russellm

  • Resolution invalid deleted
  • Status changed from closed to new

@loic84 - he may well be advised to use datetime.now, but that doesn't change the fact that using datetime.now() is a valid value for a default. It's an unusual value, to be sure, but it is a *valid* value. Off the top of my head, one possible use case would be to set up a membership model where memberships start Jan 1 2014, unless otherwise specified.

Last edited 13 months ago by russellm (previous) (diff)

comment:3 Changed 13 months ago by russellm

  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 13 months ago by russellm

  • Description modified (diff)

comment:5 Changed 13 months ago by loic84

@russellm, I get your point.

We need to make a decision at how much whitelisting we want to make in MigrationWriter since each supported type is special-cased, we don't for instance support things like frozenset.

The case of "memberships start Jan 1 2014, unless otherwise specified." can already be addressed by providing a callable that returns such date.

comment:6 Changed 13 months ago by loic84

So apparently we already special-case datetime and we document it as supported, so we ought to support timezone as well.

I'm not too sure how we can "reconstruct" the instances of tzinfo provided by pytz, but that's basically what we need.

Maybe we can provide a small utility function that works as a factory?

The failing TestCase for this issue: https://github.com/loic/django/compare/ticket21954.

Last edited 13 months ago by loic84 (previous) (diff)

comment:7 Changed 13 months ago by caulagi

Our usecase is that when creating a Blog entry, most of the time, the default value is what we want. But sometimes, we want to set created to a date in the past. So in these cases, the user will override the default timezone.now that is shown in admin for Blog entry.

Like @russellm mentioned, the stumbling thing is that it is a valid value. Otherwise, the model validation should fail, rather than the migration throwing an error, IMO.

comment:8 Changed 13 months ago by loic84

  • Cc loic@… added

@caulagi, migrations have limitations that normal models do not have: everything must be serializable, or more specifically, "reconstructable" (the system relies on "code generation" rather than "serialization" like pickle).

For instance, lambda are valid and pretty common in models, especially for default values, but these can't be supported by migrations; being valid for model validation doesn't automatically make something usable by migrations.

Now for the specific case of datetime, the system already goes out of its way to add support for it, but trips because of timezone support. We currently rely on the repr of datetime objects, but when a tzinfo is present, the repr isn't valid Python. The challenge is now, how to make datetime objects with tzinfo "reconstructable".

comment:9 Changed 13 months ago by caulagi

  • Cc caulagi added

comment:10 Changed 13 months ago by andrewgodwin

There's no way to make datetime objects with timezones reconstructable, unfortunately, as the spec for what can be in tzinfo is pretty sparse. The best I can do here is to either a) ignore the timezone, which seems like a bad option, or b) raise a nice error when this is detected and point out that you can use a callable.

I'm going to implement b).

comment:11 Changed 13 months ago by Andrew Godwin <andrew@…>

  • Owner set to Andrew Godwin <andrew@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 97a8fd4682b27a3a9f280e661e58db4bae55590a:

Fixed #21954: Raise nice error when serializing datetimes with timezones

Note: See TracTickets for help on using tickets.
Back to Top