ahawker / django-ulid

Universally Unique Lexicographically Sortable Identifier (ULID) support in Django
Apache License 2.0
42 stars 12 forks source link

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

Open opentyler opened 4 years ago

opentyler commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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

ahawker commented 3 years ago

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 commented 2 years ago

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

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

klloveall commented 2 years ago

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.