Migrations error

[codingjoe]

After the update from 1.5.5 to 1.5.6 this error happens whenever I run makemigrations.
Apparently the django can't handle the timefield changes.
Btw, we're talking 1.7.* I did not try 1.8.*

Traceback (most recent call last):
  File "./manage.py", line 11, in <module>
    execute_from_command_line(sys.argv)
  File "env/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "env/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "env/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "env/lib/python2.7/site-packages/raven/contrib/django/management/__init__.py", line 41, in new_execute
    return original_func(self, *args, **kwargs)
  File "env/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "env/lib/python2.7/site-packages/django/core/management/commands/makemigrations.py", line 124, in handle
    self.write_migration_files(changes)
  File "/env/lib/python2.7/site-packages/django/core/management/commands/makemigrations.py", line 152, in write_migration_files
    migration_string = writer.as_string()
  File "env/lib/python2.7/site-packages/django/db/migrations/writer.py", line 131, in as_string
    operation_string, operation_imports = OperationWriter(operation).serialize()
  File "env/lib/python2.7/site-packages/django/db/migrations/writer.py", line 88, in serialize
    arg_string, arg_imports = MigrationWriter.serialize(arg_value)
  File "env/lib/python2.7/site-packages/django/db/migrations/writer.py", line 333, in serialize
    return cls.serialize_deconstructed(path, args, kwargs)
  File "env/lib/python2.7/site-packages/django/db/migrations/writer.py", line 239, in serialize_deconstructed
    arg_string, arg_imports = cls.serialize(arg)
  File "env/lib/python2.7/site-packages/django/db/migrations/writer.py", line 409, in serialize
    "topics/migrations/#migration-serializing" % value
ValueError: Cannot serialize: <class django.db.models.fields.NOT_PROVIDED at 0x105702050>
There are some values Django cannot serialize into migration files.
For more, see https://docs.djangoproject.com/en/dev/topics/migrations/#migration-serializing

[ewjoachim]

Same for me

[tsantor]

Same for me on Django 1.8.3 with django-extensions==1.5.6....had to go back to django-extensions==1.5.5 to stop it from happening.

[ewjoachim]

As far as I can tell, I'd say it might well be linked to these lines in the changelog ?

Improvement: CreationDateTimeField, use auto_now_add instead of default ModificationDateTimeField
Improvement: ModificationDateTimeField, use auto_now instead of pre_save method

[codingjoe]

@ewjoachim that's what I thought. I checked out the commit, but didn't find anything yet.

[ewjoachim]

(Due to the bugs solved in 1.5.6, it seems I can't find a working version of shell_plus --notebook with my version of django if not for 1.5.6, so I can't just go back to 1.5.5 . If I can get some time, I'll try to see what's wrong...)

[trbs]

Can anybody provide a reproducible test case ?

This is really needed for a fix.

[ewjoachim]

I still don't know if I'll have enough time to fork, write a test (as far as I could tell, it probably involves either a new testapp or adding migrations to the existing testapp).

The idea of the test would be :

1. checkout the 1.5.5, manage.py makemigrations on the testapp,
2. copy the migrations files. checkout 1.5.6
3. add the migrations file to the testapp and makemigrations again (this way, we're now testing an app with migrations)
4. add a minimal test involving a ModificationDateTimeField, it will go through the migrations and (I guess) reproduce the error

[ewjoachim]

... And I just realized that what's failing is the makemigrations not the migrate. So, it's not about creating a testcase to reproduce the error, but we have to have migrations in testapp in the first place.

For every new version that has a change in the fields definition, we need to call makemigrations and if it fails, it means the feature is not ready to be commited yet (as would a SyntaxError).

What we CAN add, though, is a test that tests that migrations were created (for example, call_command("makemigrations") and check that it says it's already up to date)

(and make sure every field is defined at least once in a model in testapp, even i there's no associated test)

[zeraien]

Here is the problem:
1.5.5...1.5.6#diff-8e3fa7c3b02c3419c7374340925fe214L202

[Kobold]

+1

[trbs]

I still don't have this problem in any of my own projects.

If somebody can put a tar ball somewhere with a sample project that actually has this failure when you upgrade from 1.5.5 to 1.5.6 in virtualenv that would help.

[ryananguiano]

I ran into this issue as well, and I actually think that kwargs.setdefault('default', datetime_now) is better than kwargs.setdefault('auto_now_add', True) because I may want to add items retroactively, and auto_now_add does not allow me to set the created field. I don't know everyone's use cases but that is one concern that I have.

[smlz]

Got the same problem here: staying with 1.5.5 for now

Rhetoric question: Introducing a change with a patch level update, that requires possibly hundreds of users to migrate their data bases, really?

I am not really sure if this is a good idea, even if the automatic migration generation works without errors.

[smlz]

I created a sample app to illustrate the problem:
https://github.com/smlz/tsmtest

[trbs]

Thanks for the test case !

Please look at 1d5a86e which changes the deconstruct() method to be able to create a migration file.

@smlz I completely agree, having mandatory migration should have not been in a patch release version. Due to the (shortage of) available resources (time, etc) on my part this has "slipped" through. My apogee's for that, it should not had happened..

@ALL; please test the above patch to make sure it fixes everybody's problem.
@ALL; please give some feedback on what should happen now; (a) commit the fix (b) rollback the change and release another patch version so people can skip the migration (and discuss if it should be in the next major version).

[smlz]

@trbs no worries, we sure appreciate your work!

As for the for the fix: the generation of migration files now works seamlessly, for the little test app, as well as for our 'real' application here.

But there is a but, because now we get regression errors when running our unit tests. Somehow the semantics of 'auto_add_now' and 'default=datetime.now()' are slightly different. I did not figure out yet, what the problem is, and it might take me some time to do the research on this.

One thing I made sure is though, is that it is really the CreationDateTimeField and ModificationDateTimeField change, that caused the problem, by just applying the two commits (818f602, 1d5a86e) to 1.5.5.

Personally, I would rollback the change, quickly have a 1.5.7 release, and then quietly see how to integrate this change without disrupting existing installations.

[trbs]

Unless there are other concerns raised, I will try to release 1.5.7 next Sunday or Monday. Which will be the current master but with the problematic patch rolled back.

[im-n1]

I can confirm that current master fixes the bug. Thanks!

[trbs]

I just release 1.5.7 it contains the fix but does not rollback to before since it seems that will cause another migration for people.

Happy to try and help to solve any remaining problems for people.

[ewjoachim]

Hi,

So yes it solved the migrations issue, but I have the same problem as @smlz concerning my unit tests that are broken.

As far as I understand, it's because we now fall into this warning's scope, meaning that ALL people that had fixtures that did not have all dates explicitely stated (for example, if you don't care), it won't work anymore.

With this in mind, maybe a "rollback" (or maybe just having the fields available as implemented before, even if with a different name) would help. Now that the new version is out with its migrations, we're probably not going to undo it because that's going to break things, so maybe the best is to stop modifying this field and have a new one with the other implementation.

Another possibility is to push a legacy version in a new package just for the people who need it (and don't want to update all their fixture files), with explicitely no support for it. But at least, it will allow people who need time to upgrade their fixtures not to be tied with 1.5.5 (which has other bugs elsewhere that were properly removed).

I'm under the impression that auto_add and auto_add_now are more or less not well advised solutions to solve problems.

Related prose : https://code.djangoproject.com/ticket/22995

[ewjoachim]

(Solved everything by taking the source of the 1.5.5 Creation and Modification Fields definition in our own code, and not using Django Extensions for that part 😕 )

[trbs]

It's a sensitive subject in the ticket as well :)

I remembered it as that auto_now and auto_add_now got more or less reinstated (and not deprecated) which is why it seemed a good idea at the time to use the solution that required the least custom code not realizing the migration and other side effects :(

The primary reason for now rolling back right away was indeed that I don't know how many people already made a migration in their project and a rollback would force yet another one.

I'm happy to add a second set of fields using the old version.

Thinking that it would probably also work to have a flag in settings.py to select which one you want as the default, so that people using 1.5.8 can also set settings.CreationDateTimeFieldUseAutoAdd = False and settings.ModificationDateTimeFieldUseAutoAddNow = False (or something similar) and have everything work again like in 1.5.5

Would that work for you guys ?

[ewjoachim]

It means that for those who migrated to 1.5.7, they will need a new migration to go back to the 1.5.8 with those settings as False... But I'm not sure there's another solution ...

Joachim Jablon

Le 21 sept. 2015 à 17:56, trbs notifications@github.com a écrit :

It's a sensitive subject in the ticket as well :)

I remembered it as that auto_now and auto_add_now got more or less reinstated (and not deprecated) which is why it seemed a good idea at the time to use the solution that required the least custom code not realizing the migration and other side effects :(

The primary reason for now rolling back right away was indeed that I don't know how many people already made a migration in their project and a rollback would force yet another one.

I'm happy to add a second set of fields using the old version.

Thinking that it would probably also work to have a flag in settings.py to select which one you want as the default, so that people using 1.5.8 can also set settings.CreationDateTimeFieldUseAutoAdd = False and settings.ModificationDateTimeFieldUseAutoAddNow = False (or something similar) and have everything work again like in 1.5.5

Would that work for you guys ?

—
Reply to this email directly or view it on GitHub.

[smlz]

We sure have a big mess by now... ;-)

Thinking of it now, having a external library that might have a direct influence on the data base layout of your models, is it really a good idea? I don't know.

But it sure is not be a lot of code in this case:

from django.db import models
from django.utils.timezone import now

class TimeStampedModel(models.Model):
    created = models.DateTimeField(_('created'), default=now,
                                   editable=False, blank=True)
    modified = models.DateTimeField(_('modified'), default=now,
                                    editable=False, blank=True)

    def save(self, *args, **kwargs):
        self.modified = now()
        super(TimeStampedModel, self).save(*args, **kwargs)

    class Meta:
        abstract = True

With this bit of code, I can remove django-extensions from the production dependencies altogether, and just keep it as as developer tool. This is also where this library really rocks: runserver_plus and shell_plus. And as a dev tool, it doesn't always need to be 100% production ready.

[codingjoe]

@ewjoachim you should create a migration based on the latest version and include all previous migrations. Shouldn't that allow 1.5.6 and 1.5.7 users to update?

@smlz I disagree. django-extensions is one of the best maintained packages out there. I wish my production code was that good. I didn't run into that problem, because my tests caught the problem and I just didn't upgrade. I think there is no problem in 3rd party code unless you mindlessly update without reviewing the changelog.

[ewjoachim]

@codingjoe Yes but the goal is not to add a new migration. But of course, if you're ready to add new migrations, it's a piece of cake :)

[smlz]

@codingjoe: don't over-interpret one random dude moving one single library from prod- to dev-dependencies for something it is not. Sometimes it is just simpler to add 15 lines of code than going through the hassle of generating 20 migration files and fixing 50+ failing unit tests. Potentially not only for me, but also somebody else. Hence the comment.

And dropping a dependency is always a good thing, because every time it happens, a cute little baby panda is born 🐼

[codingjoe]

😸

[trbs]

@codingjoe thanks for the complement. I think django-extensions is not nearly as good as for example Django, specially in the documentation department, but we manage :)

@smlz I personally tent to avoid depending on third party models. Since I like to be in control of and changes and upgrade paths. But in my professional projects I have a team to help me with managing that kind of stuff :)

More broader, I don't feel like we have a strict definition or policy on what would constitute something suitable for including in Django Extensions. One could argue (abstract) models and fields should not be included, on the other hand you could also say we are just providing 'more batteries' ;)

[trbs]

Back on topic; is there a (strong) need for the old fields to be restored with a different name for some kind of backwards compatibility ?

[ewjoachim]

In my opinion, it's easy enough to let people who have the issue either rewrite their fixtures or use @smlz 's snippet to go back to the previous behaviour...?

[trbs]

That's what I was hoping for. Having two versions of the field has it's own (nasty) side effects, but if there was some dire need we could have accommodated with some deprecation schedule.

Closing this now, sorry for the mess everybody, lets hug and move on :)