8000 Error when creating ordered model in migration · Issue #253 · django-ordered-model/django-ordered-model · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Error when creating ordered model in migration #253

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.

Sign up for GitHub

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

Jump to bottom
Open
embeddedt opened this issue Aug 17, 2021 · 5 comments
Open

Error when creating ordered model in migration #253

embeddedt opened this issue Aug 17, 2021 · 5 comments

Comments

@embeddedt
Copy link

In the past I've made migrations which look like this:

from django.db import migrations

# excerpt from models.py
class MyModel(OrderedModel):
    pass
# end excerpt

class Migration(migrations.Migration):
    def create_crossword_activity(apps, schema_editor):
        MyModel = apps.get_model('frontend', 'MyModel')
        MyModel.objects.create(name='Crossword')
    dependencies = [
        ('frontend', '0008_auto_20210730_1445'),
    ]

    operations = [
        migrations.RunPython(create_crossword_activity)
    ]

The previous versions of this migration which were made before MyModel inherited from OrderedModel work fine.

However, when I make a new version of the migration with the OrderedModel version, it fails with the following error:

django.db.utils.IntegrityError: NOT NULL constraint failed: frontend_mymodel.order

I suspect the reason this occurs is because the OrderedModelManager does not have use_in_migrations set to True.
@shuckc
Copy link
Member
shuckc commented Feb 22, 2022

Hi embeddedt - did you get any further with this? I would be happy to add a default True value if you can confirm it helps.

@embeddedt
Copy link
Author

I ended up working around it for now by adding order=0 as one of the parameters to the create command, and then reordering the models afterward.

Unfortunately, it seems that setting use_in_migrations to True does not get rid of the issue as I had hoped.

@HengJunXi
Copy link

I suspect that without use_in_migrations=True, the OrderedModelManager will not be used in the RunPython. Hence the order field is not set automatically. The workaround or appropriate things to do is to set the order manually in the RunPython, which is exactly what is done here at the end.

Alternatively, if use_in_migrations=True is set (and you can't unset it), you will need to do setattr(MyModel, "order_field_name", "order") and setattr(MyModel, "order_with_respect_to", "<your-order-with-respect-to>"), after you get the model through apps.get_model. This is because these 2 attributes defined in the model is not available through apps.get_model (which return ModelBase object), but the OrderedModelManager is accessing them.
This hacky workaround does not work for bulk_create (which I am currently struggling with) and maybe other things. The only way to "fix" this, might be open PR for django-ordered-model to move order_field_name & order_with_respect_to to Meta (if possible).

@shuckc
Copy link
Member
shuckc commented Mar 15, 2023

Thanks for the comments. I did try sometime last year to write a unit test that covered running a migration that created a model, based on the Django unit tests, but it was superbly complicated and I couldn't justify the time to fully debug what was happening. Without a decent test case I left the ticket.

The Meta class (regrettably) is the right place for this configuration. Django introduced similar, but not identical functionality using the Meta field order_with_respect_to . So we would need to rename our config fields, perhaps Meta.ordered_model_with_respect_to and Meta.ordered_model_field?

I would certainly review and work towards a patch to support setting these values on the Meta, and fixing up our docs and checks to validate them. We could use the old fields on the model as a fallback, raising a warning to help migrate. Especially if that is all that is actually required to make our manager work during a migration.

@HengJunXi
Copy link

Currently, it is impossible to add custom attributes to Meta. There are discussion about this in https://code.djangoproject.com/ticket/5793, which suggested Django core to support this. However, it's been a while since the last activity.

I've tried monkey patching on the Django core Meta options, but don't think it will work consistently since there is no guarantee that the library can apply the monkey patch before Django is loaded. It will also be hard to maintain given that Meta is heavily used in Django, and might break things (or other libraries) due to the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@shuckc @HengJunXi @embeddedt
0