[Django] #29738: Django can't serialize DateTimeTZRange(lower=None, upper=None, bounds='[)')
Django
---------------------------------------+------------------------
Reporter: grahammayer | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
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, '[)')
--
Ticket URL: <https://code.djangoproject.com/ticket/29738>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: grahammayer | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: rangefield, | Triage Stage: Accepted
postgresql, psycopg2, migrations |
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Nick Pope):
* component: Migrations => contrib.postgres
* needs_tests: 0 => 1
* easy: 0 => 1
* keywords: => rangefield, postgresql, psycopg2, migrations
* needs_docs: 0 => 1
* stage: Unreviewed => Accepted
Old description:
New description:
--
Comment:
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
[https://docs.djangoproject.com/en/2.1/topics/migrations/#serializing-
values documentation].
So there are a few options:
a. Can you get away with using {{{default=None, null=True}}}?
b. Does {{{default=(None, None)}}} work instead? (I believe this will be
limited to the
[http://initd.org/psycopg/docs/extras.html#psycopg2.extras.Range default
bounds]: `[)`)
c. 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:
{{{#!python
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:
{{{#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:1>
Django
Reporter: grahammayer | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Simon Charette):
* keywords: rangefield, postgresql, psycopg2, migrations =>
Old description:
New description:
--
Comment:
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
[https://github.com/django/django/blob/570402ffd7efd529eb597d86525b80a6fbfca0e7/django/db/migrations/serializer.py#L273-L327
that long series of if] by iterating over the registry items.
{{{#!python
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 `abc`s.
{{{#!python
from abc import ABC
class Deconstructable(ABC):
@classmethod
def __subclasshook__(cls, C):
return hasattr(C, 'deconstruct')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:2>
Django
Reporter: grahammayer | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
postgresql psycopg2 migrations |
removed |
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* keywords: => rangefield postgresql psycopg2 migrations removed
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:3>
Django
Reporter: grahammayer | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Nick Pope:
Old description:
New description:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:4>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 2.0
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Can Sarıgöl
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:5>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 2.0
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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(...)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:6>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 2.0
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
[https://github.com/django/django/pull/10394]
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:7>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* component: contrib.postgres => Migrations
* needs_tests: 1 => 0
* easy: 1 => 0
* needs_docs: 1 => 0
* has_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:8>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by 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'>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:9>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
New [https://code.djangoproject.com/ticket/29738 PR]. Still needs
improvement. I'm not sure the general approach is suitable.
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:10>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Can Sarıgöl):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/10673 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:11>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"7d3b3897c1d7b1ae4dfea6ae0d4f431d3e3dec1c" 7d3b3897]:
{{{
#!CommitTicketReference repository=""
revision="7d3b3897c1d7b1ae4dfea6ae0d4f431d3e3dec1c"
Refs #29738 -- Allowed registering serializers with MigrationWriter.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:12>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Component: Migrations | Version: 2.0
Severity: Normal | Resolution: fixed
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e192223ed996ed30fe83787efdfa7f2be6b1a2ee" e192223e]:
{{{
#!CommitTicketReference repository=""
revision="e192223ed996ed30fe83787efdfa7f2be6b1a2ee"
Fixed #29738 -- Allowed serializing psycopg2 range types in migrations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:13>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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():
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:14>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* cc: jon.dufresne@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:15>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
The regression is being tracked in #30111
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:16>
Django
Reporter: Graham Mayer | Owner: Can
| Sarıgöl
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: rangefield | Triage Stage: Accepted
postgresql psycopg2 migrations |
removed |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"52f6927d7fb7a4dca40afce0391d018b4c34dd6d" 52f6927]:
{{{
#!CommitTicketReference repository=""
revision="52f6927d7fb7a4dca40afce0391d018b4c34dd6d"
Refs #29738 -- Added test for serializing psycopg2's NumericRange with
DecimalRangeField in migrations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29738#comment:17>