Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with Django 1.7 migrations? #103

Closed
jannon opened this issue Apr 1, 2015 · 16 comments
Closed

Problem with Django 1.7 migrations? #103

jannon opened this issue Apr 1, 2015 · 16 comments
Assignees
Labels

Comments

@jannon
Copy link

jannon commented Apr 1, 2015

I just ran my first migration on a model that has an hstore field in it. I had changed the default of a datetime field within the hstore field (because of encode/django-rest-framework#2687) and the migration blew up with:

ValueError: Cannot alter field batches.Collection.end_date into app.MyModel.end_date - they do not properly define db_type (are you using PostGIS 1.5 or badly-written custom fields?)

So I took a look at what the migration was doing:

migrations.AlterField(
            model_name='mymodel',
            name='end_date',
            field=django_hstore.virtual.VirtualField(default=datetime.date(2015, 1, 1)),
        )

It seems like the django_hstore.virtual.VirtualField fields shouldn't be included in the migration. I wanted to check and see if others had this issue or if I was just missing something before I looked in to how we might avoid this (other than manually editing the migration every time)

@nemesifier
Copy link
Member

Oh no again.
I tried reporting this problem last year:
https://code.djangoproject.com/ticket/23159
I also sent a pull request: https://github.com/django/django/pull/3013/files
But I was adviced to just set db_type as None in the virtual fields by @andrewgodwin and the issue was set as won't fix.
With some more tricks I was able to make the basic cases just work fine, but the solutions I found might not work in all cases, like yours.

So, I don't really know how to proceed. Apart from just deleting part of the migration manually, which obviously sucks but will work if in a hurry, we could do two things:

  • insist to reopen the issue and find a way to tell the django migration framework to ignore virtual fields
  • try to find another workaround on this particular issue

Federico

@jannon
Copy link
Author

jannon commented Apr 2, 2015

Bah! Okay, yeah I doubt that we'd get them to reopen the issue, especially given Django1.8's new HStoreField. Until I get a chance to see how 1.8 works, or until the DRF bug gets fixed, I think I'll just manually edit the migrations

@nemesifier
Copy link
Member

Ok @jannon.

@timgraham
Copy link

Can you paste the model field before and after?

@nemesifier
Copy link
Member

@jannon could you answer to @timgraham's question please?

@jannon
Copy link
Author

jannon commented Apr 10, 2015

Before:

class MyModel(models.Model):
    hstore_field = hstore.DictionaryField(schema=[
        {
            'name': 'end_date',
            'class': 'DateField',
            'kwargs': {'null': True, 'blank': True}
        }
    ], null=True, blank=True)

After:

class MyModel(models.Model):
    hstore_field = hstore.DictionaryField(schema=[
        {
            'name': 'end_date',
            'class': 'DateField',
            'kwargs': {'null': True, 'blank': True, 'default': date(2015, 1, 1)}
        }
    ], null=True, blank=True)

@timgraham
Copy link

I think the migration that's generated is correct, but Django doesn't seem to have good support for ignoring virtual fields in the schema editor. You might be able to workaround it by adding a rel attribute to django_hstore.virtual.VirtualField with the appropriate sub-attributes so that you reach the no-op branch in the schema editor.

This might be properly fixed if Django gains support for composite fields in 1.9, but it seems like a ticket is justified (a separate one from what was originally reported).

@nemesifier
Copy link
Member

thx for the info @timgraham, is there any link where we can find more information? A mailing list thread, a ticket, or anything else?

@timgraham
Copy link

More information about what? Composite field DEP is here if that's what you meant: django/deps#12

@nemesifier
Copy link
Member

Exactly, thank you.

@nemesifier
Copy link
Member

@timgraham & @jannon I pushed a possible fix for the ValueError in e3ff9bf, the problem is I don't have a clue of how to make a failing test case ... 👎 any hints?

@nemesifier
Copy link
Member

After some manual testing it turned out the code of e3ff9bf was not effective.

Thanks to @apollo13's help I come up with 618f1ff which correctly eliminates the ValueError, the problem is I still can't figure out how to test this automatically.

@timgraham
Copy link

You might take inspiration from Django's test suite: https://github.com/django/django/blob/master/tests/schema/tests.py

@nemesifier
Copy link
Member

It should be fixed.

Thanks to everyone who helped out: @jannon for reporting, @timgraham and @apollo13 for helping me with suggestions.

I opened a new thread on django-developers here: https://groups.google.com/forum/#!topic/django-developers/_FmJRK3sJGs

I received another suggestion by @MarkusH which I'll try out before closing this issue.

@jannon
Copy link
Author

jannon commented Apr 22, 2015

Federico,
This works great!
It'll be nice if the the virtual=True suggestion will even help clean things up a bit

nemesifier added a commit that referenced this issue Apr 23, 2015
Avoid leaving migration files if the method test_migration fails
or tests are interrupted.

Related to #103
@nemesifier
Copy link
Member

I mark this closed for the moment, but I'll keep following the mailing list for suggestions.

nemesifier added a commit that referenced this issue Jun 28, 2015
* use the new virtual=True in _meta.add_field()
* substitute a copy of _meta.fields instead of mutating it
* test added for #103 is obsolete in django 1.8

closes #118
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants