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.

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

Using ULID field for primary key causing migration to keep being recreated #65

Open
opentyler opened this issue Oct 22, 2020 · 5 comments · May be fixed by #150
Open

Using ULID field for primary key causing migration to keep being recreated #65

opentyler opened this issue Oct 22, 2020 · 5 comments · May be fixed by #150

Comments

@opentyler
Copy link

opentyler commented Oct 22, 2020 •
edited
Loading

I'm using ULID as the primary key for a couple of my models, as well as ULID for a couple non-PK keys. When i run python manage.py makemigrations, a migration is made attempting to redeclare the ulid fields being used as pk, but the non-pk fields aren't affected which is good. If I try to run the migration, it results with an error, reproduced below.

Not sure why django wants to keep creating this migration, as its the exact same operation as the initial migration.

models.py:

class Document(MP_Node):
    id = ULIDField(default=default, primary_key=True, editable=False)
    tree_root_pk = ULIDField(null=True, blank=True)

class Tag(models.Model):
    id = ULIDField(default=default, primary_key=True, editable=False)

Initial migration:

migrations.CreateModel(
    name='Tag',
    fields=[
        ('id', django_ulid.models.ULIDField(default=ulid.api.api.Api.new, editable=False, primary_key=True, serialize=False)),

migrations.CreateModel(
    name='Document',
    fields=[
        ('id', django_ulid.models.ULIDField(default=ulid.api.api.Api.new, editable=False, primary_key=True, serialize=False)),
        ('tree_root_pk', django_ulid.models.ULIDField(blank=True, null=True)),

Migration that keeps getting recreated every time I run python manage.py makemigrations:

operations = [
    migrations.AlterField(
        model_name='document',
        name='id',
        field=django_ulid.models.ULIDField(default=ulid.api.api.Api.new, editable=False, primary_key=True, serialize=False),
    ),
    migrations.AlterField(
        model_name='tag',
        name='id',
        field=django_ulid.models.ULIDField(default=ulid.api.api.Api.new, editable=False, primary_key=True, serialize=False),
    ),
]

The error I get if I try to run this superfluous migration:

Operations to perform:
  Apply all migrations: admin, auth, contenttypes, djstripe, documents, sessions, users
Running migrations:
  Applying documents.0003_auto_20201022_1945...Traceback (most recent call last):
  File "manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "lib/python3.7/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "lib/python3.7/site-packages/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "lib/python3.7/site-packages/django/core/management/base.py", line 328, in run_from_argv
    self.execute(*args, **cmd_options)
  File "lib/python3.7/site-packages/django/core/management/base.py", line 369, in execute
    output = self.handle(*args, **options)
  File "lib/python3.7/site-packages/django/core/management/base.py", line 83, in wrapped
    res = handle_func(*args, **kwargs)
  File "lib/python3.7/site-packages/django/core/management/commands/migrate.py", line 233, in handle
    fake_initial=fake_initial,
  File "lib/python3.7/site-packages/django/db/migrations/executor.py", line 117, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "lib/python3.7/site-packages/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "lib/python3.7/site-packages/django/db/migrations/executor.py", line 245, in apply_migration
    state = migration.apply(state, schema_editor)
  File "lib/python3.7/site-packages/django/db/migrations/migration.py", line 124, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "lib/python3.7/site-packages/django/db/migrations/operations/fields.py", line 249, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "lib/python3.7/site-packages/django/db/backends/base/schema.py", line 565, in alter_field
    old_db_params, new_db_params, strict)
  File "lib/python3.7/site-packages/django/db/backends/postgresql/schema.py", line 154, in _alter_field
    new_db_params, strict,
  File "lib/python3.7/site-packages/django/db/backends/base/schema.py", line 678, in _alter_field
    old_default = self.effective_default(old_field)
  File "lib/python3.7/site-packages/django/db/backends/base/schema.py", line 303, in effective_default
    return field.get_db_prep_save(self._effective_default(field), self.connection)
  File "lib/python3.7/site-packages/django/db/backends/base/schema.py", line 282, in _effective_default
    default = field.get_default()
  File "lib/python3.7/site-packages/django/db/models/fields/__init__.py", line 829, in get_default
    return self._get_default()
TypeError: new() missing 1 required positional argument: 'self'
@gmjosack
Copy link

I ran into this as well. I tracked it down to the ulid.new function returning an object which I guess makes django unhappy. if i make my own function and coerce it to a string then everything works as expected

from django_ulid.models default as default_ulid

def new_ulid():
    return str(default_ulid())

then pass new_ulid to default

@opentyler
Copy link
Author

opentyler commented Nov 21, 2020 •
edited
Loading

Awesome, thanks for the heads up. This solution worked beautifully for me too. Cheers!

@ahawker
Copy link
Owner

ahawker commented Nov 22, 2020

My apologies for the delay, I missed the notifications for this issue.

A few versions back, support for monotonic randomness was added to ulid-py. This made the randomness implementation pluggable and the exported API is now bound methods on a constant object instead of pure functions as before.

I'll look to see if I can expose a pure function from django-ulid to make this easier for downstream consumers.

@fdobrovolny
Copy link
Contributor

There is even a bug report to Django for it from someone.

https://code.djangoproject.com/ticket/32689

@klloveall
Copy link

I understand this is an issue originating from a downstream project, however this prevents anyone from using django-ulid without doing the workaround given by @gmjosack above and just cost me a few hours trying to figure out why in the world these identical migrations were being created.

I'd be happy to provide a patch for this so this works out-of-the-box again if you can point me in the correct direction @ahawker.

kwood added a commit to kwood/django-ulid that referenced this issue Feb 23, 2023
@kwood
Make default a pure function
3a6e98e 
Fixes ahawker#65
@kwood kwood linked a pull request Feb 23, 2023 that will close this issue
Open
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

Successfully merging a pull request may close this issue.

Make default a function instead of reference kwood/django-ulid
5 participants
@ahawker @gmjosack @fdobrovolny @klloveall @opentyler