Django can't serialize DateTimeTZRange(lower=None, upper=None, bounds='[)')

[Graham Mayer]

Tried to use DateTimeTZRange(lower=None, upper=None, bounds='[)') as a default for a model field and get the following error when running python manage.py makemigrations:

 Traceback (most recent call last):
  File "manage.py", line 12, in <module>
    execute_from_command_line(sys.argv)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/__init__.py", line 371, in execute_from_command_line
    utility.execute()
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/__init__.py", line 365, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/base.py", line 335, in execute
    output = self.handle(*args, **options)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/commands/makemigrations.py", line 172, in handle
    self.write_migration_files(changes)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/core/management/commands/makemigrations.py", line 210, in write_migration_files
    migration_string = writer.as_string()
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/writer.py", line 151, in as_string
    operation_string, operation_imports = OperationWriter(operation).serialize()
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/writer.py", line 110, in serialize
    _write(arg_name, arg_value)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/writer.py", line 74, in _write
    arg_string, arg_imports = MigrationWriter.serialize(_arg_value)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/writer.py", line 279, in serialize
    return serializer_factory(value).serialize()
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/serializer.py", line 203, in serialize
    return self.serialize_deconstructed(path, args, kwargs)
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/serializer.py", line 90, in serialize_deconstructed
    arg_string, arg_imports = serializer_factory(arg).serialize()
  File "/home/grahammayer/logimeter/logimeter/lib/python3.6/site-packages/django/db/migrations/serializer.py", line 370, in serializer_factory
    "topics/migrations/#migration-serializing" % (value, get_docs_version())
ValueError: Cannot serialize: DateTimeTZRange(None, None, '[)')

[Nick Pope]

Presumably you are using default=DateTimeTZRange(lower=None, upper=None, bounds='[)')? Could you provide a simple example project?

The problem is that psycopg2.extras.DateTimeTZRange cannot be serialized into a migrations, see ​documentation.

So there are a few options:

1. Can you get away with using default=None, null=True?
2. Does default=(None, None) work instead? (I believe this will be limited to the ​default bounds: [))
3. You need to make DateTimeTZRange deconstructible which will allow for other bounds to be defined.

We should be able to get away with making these range objects deconstructible as we only expect them to take simple arguments that are already supported by the serializer.

Here is an example:

from psycopg2.extras import DateTimeTZRange

from django.contrib.postgres.fields import DateTimeRangeField
from django.utils.deconstruct import deconstructible


DateTimeTZRange = deconstructible(DateTimeTZRange, path='psycopg2.extras.DateTimeTZRange')


class RangeTestModel(models.Model):
    a = DateTimeRangeField(
        default=None,
        null=True,
    )
    b = DateTimeRangeField(
        default=(None, None),
        null=False,
    )
    c = DateTimeRangeField(
        default=DateTimeTZRange(None, None, '[]'),
        null=False,
    )

I managed to produce the following migration:

import django.contrib.postgres.fields.ranges
from django.db import migrations, models
import psycopg2.extras


class Migration(migrations.Migration):

    dependencies = [
    ]

    operations = [
        migrations.CreateModel(
            name='RangeTestModel',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('a', django.contrib.postgres.fields.ranges.DateTimeRangeField(default=None, null=True)),
                ('b', django.contrib.postgres.fields.ranges.DateTimeRangeField(default=(None, None))),
                ('c', django.contrib.postgres.fields.ranges.DateTimeRangeField(default=psycopg2.extras.DateTimeTZRange(None, None, '[]'))),
            ],
        ),
    ]

(Disclaimer: I have only attempted to produce migrations and not actually called RangeTestModel.objects.create()...)

So in summary, I think that we only need to add some documentation to explain how to handle defaults for range fields and tests to ensure that it continues to work.

[Simon Charette]

A solution could to make django.db.migration.serializer offer a way to register serializers for types and have django.contrib.postgres.App.ready() be in charge of registering the ones to handle psycopg2 range types.

If the registry uses an OrderedDict we should be able to replace ​that long series of if by iterating over the registry items.

registry = OrderedDict([
    (models.Field, ModelFieldSerializer),
    ...
])

def serializer_factory(value):
    if isinstance(value, Promise):
        value = str(value)
    elif isinstance(value, LazyObject):
        # The unwrapped value is returned as the first item of the arguments
        # tuple.
    value = value.__reduce__()[1][0]

    for type_, serializer_cls in registry.items():
        if isinstance(value, type_):
            return serializer_cls(value)
    raise ValueError(...)

The hasattr and checks could be performed using abcs.

from abc import ABC
class Deconstructable(ABC):
    @classmethod
    def __subclasshook__(cls, C):
        return hasattr(C, 'deconstruct')

[Can Sarıgöl]

Can I change serializer_factory like this? so that all unserialized values can be tried to be available.

def serializer_factory(value):
    ...


    from django.utils.deconstruct import deconstructible
    deconstructible_value = deconstructible(
        value.__class__,
        path=value.__module__ + "." + value.__class__.__name__
    )
    value = deconstructible_value(value.__init__)
    return DeconstructableSerializer(value)

    raise ValueError(...)

[Can Sarıgöl]

​https://github.com/django/django/pull/10486

[Tim Graham]

I tested the patch and noticed that while it worked for the use case in the ticket, it put import psycopg2._range in the migration rather than from psycopg2.extras import DateTimeTZRange that's in the models file. Maybe there's no way around that.

[Can Sarıgöl]

When I check DateTimeTZRange, the result is like below. Is there any problem or does this usage make a problem?

>>> from psycopg2.extras import DateTimeTZRange
>>> print(DateTimeTZRange().__class__)
<class 'psycopg2._range.DateTimeTZRange'>

[Tim Graham]

New PR. Still needs improvement. I'm not sure the general approach is suitable.

[Can Sarıgöl]

​New PR Could you review this last pr? i don't know why but there is a problem in "pull-requests-bionic/database=sqlite3,label=bionic-pr,python=python3" test. I checked in my local django-box and it was good.

[Tim Graham <timograham@…>]

In 7d3b3897:

Refs #29738 -- Allowed registering serializers with MigrationWriter.

[Tim Graham <timograham@…>]

In e192223e:

Fixed #29738 -- Allowed serializing psycopg2 range types in migrations.

[Jon Dufresne]

After running a bisect, I've found that this change caused a regression for my project. This occurs when running management commands. It looks like from django.db.migrations.writer import MigrationWriter is not safe at the module level when django.setup() is called. Here is the stack trace:

Traceback (most recent call last):
  File ".../myproject/manage.py", line 12, in <module>
    execute_from_command_line(sys.argv)
  File ".../django/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File ".../django/django/core/management/__init__.py", line 357, in execute
    django.setup()
  File ".../django/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File ".../django/django/apps/registry.py", line 91, in populate
    app_config = AppConfig.create(entry)
  File ".../django/django/apps/config.py", line 116, in create
    mod = import_module(mod_path)
  File "/usr/lib64/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File ".../django/django/contrib/postgres/apps.py", line 8, in <module>
    from django.db.migrations.writer import MigrationWriter
  File ".../django/django/db/migrations/writer.py", line 10, in <module>
    from django.db.migrations.loader import MigrationLoader
  File ".../django/django/db/migrations/loader.py", line 8, in <module>
    from django.db.migrations.recorder import MigrationRecorder
  File ".../django/django/db/migrations/recorder.py", line 9, in <module>
    class MigrationRecorder:
  File ".../django/django/db/migrations/recorder.py", line 22, in MigrationRecorder
    class Migration(models.Model):
  File ".../django/django/db/models/base.py", line 99, in __new__
    app_config = apps.get_containing_app_config(module)
  File ".../django/django/apps/registry.py", line 252, in get_containing_app_config
    self.check_apps_ready()
  File ".../django/django/apps/registry.py", line 135, in check_apps_ready
    raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

This change appears to fix it for me, but I haven't had a chance to fully verify its correctness or write a test.

diff --git a/django/contrib/postgres/apps.py b/django/contrib/postgres/apps.py
index 97475de6f7..151450f4e4 100644
--- a/django/contrib/postgres/apps.py
+++ b/django/contrib/postgres/apps.py
@@ -5,7 +5,6 @@ from psycopg2.extras import (
 from django.apps import AppConfig
 from django.db import connections
 from django.db.backends.signals import connection_created
-from django.db.migrations.writer import MigrationWriter
 from django.db.models import CharField, TextField
 from django.test.signals import setting_changed
 from django.utils.translation import gettext_lazy as _
@@ -22,6 +21,7 @@ def uninstall_if_needed(setting, value, enter, **kwargs):
     Undo the effects of PostgresConfig.ready() when django.contrib.postgres
     is "uninstalled" by override_settings().
     """
+    from django.db.migrations.writer import MigrationWriter
     if not enter and setting == 'INSTALLED_APPS' and 'django.contrib.postgres' not in set(value):
         connection_created.disconnect(register_type_handlers)
         CharField._unregister_lookup(Unaccent)
@@ -42,6 +42,7 @@ class PostgresConfig(AppConfig):
     verbose_name = _('PostgreSQL extensions')

     def ready(self):
+        from django.db.migrations.writer import MigrationWriter
         setting_changed.connect(uninstall_if_needed)
         # Connections may already exist before we are called.
         for conn in connections.all():

[Jon Dufresne]

The regression is being tracked in #30111

[Mariusz Felisiak <felisiak.mariusz@…>]

In 52f6927:

Refs #29738 -- Added test for serializing psycopg2's NumericRange with DecimalRangeField in migrations.