Opened 11 years ago
Closed 11 years ago
#21954 closed Bug (fixed)
1.7a1 - Migration syntax error
Reported by: | caulagi | Owned by: | |
---|---|---|---|
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 )
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 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 11 years ago
Resolution: | invalid |
---|---|
Status: | closed → 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.
comment:3 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 11 years ago
Description: | modified (diff) |
---|
comment:5 by , 11 years ago
@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 by , 11 years ago
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.
comment:7 by , 11 years ago
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 by , 11 years ago
Cc: | 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 by , 11 years ago
Cc: | added |
---|
comment:10 by , 11 years ago
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 by , 11 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Hi @caulagi, the problem is that you are not using a callable for the default value of
created
but an actualdatetime
object.Try
created = models.DateTimeField(default=timezone.now)
instead; notice the missing parentheses after "timezone.now".