[Django] #29505: Impossible to set a Field's default value to a callable
Django
-------------------------------------+-------------------------------------
Reporter: Eugene | Owner: nobody
Pakhomov |
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords: default
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
I'm trying to use https://github.com/cognitect/transit-python and I
created a custom `KeywordField` for this.
The problem is that `transit.transit_types.Keyword` is a callable. Now,
even if I override `_get_default` in `KeywordField`, it still won't work
because
`django.db.backends.base.schema.BaseDatabaseSchemaEditor#effective_default`
checks the value again.
I think a simple fix would be to remove this check from
`effective_default` and immediately make a call where it's necessary
(`datetime.date` etc).
--
Ticket URL: <https://code.djangoproject.com/ticket/29505>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Eugene Pakhomov | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: default | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Changes (by Tim Graham):
* component: Database layer (models, ORM) => Migrations
* easy: 1 => 0
Comment:
I don't understand the issue. Please explain in more detail and perhaps
offer a patch to explain.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:1>
Django
Reporter: Eugene Pakhomov | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: default | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Comment (by Eugene Pakhomov):
Suppose I have some custom type like this one
{{{
#!python
class MyStr(str):
def __call__(self, other):
return self + other
}}}
and a corresponding custom field type that inherits from Django's
`TextField` and implements all necessary methods so that `MyStr` is used
everywhere instead of the regular `str`.
Currently, it's not possible without manually creating a `DatabaseWrapper`
with a custom `DatabaseSchemaEditor` because even if you handle the fact
that instances of `MyStr` are callable in the custom field type, they will
still be called in
`django.db.backends.base.schema.BaseDatabaseSchemaEditor#effective_default`.
Attached a patch that fixes this issue. Tests appear to be fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:2>
Django
Reporter: Eugene Pakhomov | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: default | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
* Attachment "29505.diff" added.
Django
argument
---------------------------------+--------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Component: Migrations | Version: 2.0
Severity: Normal | Resolution: wontfix
Keywords: default | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
I believe the proposed patch would break callable defaults. In
`effective_defautl()`, `field.get_default()` may return a callable and
`Field.get_prep_value()` won't work properly if it receives a callable
instead of a value. I guess test coverage is lacking.
I think you should change your `__call__()` method to make `other` an
optional argument.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:3>
Django
argument
---------------------------------+--------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 2.0
Severity: Normal | Resolution: wontfix
Keywords: default | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Comment (by Eugene Pakhomov):
As I said in the issue description - the types under question are not
under my control, so I cannot change `__call__()`. And my concern doesn't
have to do anything with my particular situation - I have already solved
it by setting a custom `DatabaseSchemaEditor`. My concern is that this
issue is still there, and other people will have to spend some time
debugging it.
> field.get_default() may return a callable
You're incorrect. Django itself doesn't do that. And `Model#__init__()`
implementation calls `get_default()` without checking whether it's result
is callable or not because the default implementation of `get_default()`
always does this check. If `get_default()` could return a callable,
`Model#__init__()` would not work correctly - it would set the
corresponding field in all instances to the callable, without calling it
to get the default value.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:4>
Django
argument
---------------------------------+--------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 2.0
Severity: Normal | Resolution: wontfix
Keywords: default | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+--------------------------------------
Comment (by Eugene Pakhomov):
Also, the change in title is incorrect.
Technically, it's possible to set the default value to something that is a
callable without arguments, yes. But it's impossible to also **have** this
value as the default, instead of just have it be called for a default.
Consider my example above but without the `other` argument. You still
won't be able to set an instance of `MyStr` as the default value.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:5>
Django
that'll be used as the default
---------------------------------+------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by Tim Graham):
* status: closed => new
* type: Uncategorized => Bug
* needs_tests: 0 => 1
* has_patch: 0 => 1
* resolution: wontfix =>
* stage: Unreviewed => Accepted
Comment:
After a second look, I agree with your analysis. The patch looks okay, and
the test will probably go in `tests/schema`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:6>
Django
that'll be used as the default
---------------------------------+------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 2.0
Severity: Normal | Resolution:
Keywords: default | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+------------------------------------
Changes (by Tim Graham):
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/10659 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:7>
Django
that'll be used as the default
---------------------------------+------------------------------------
Reporter: Eugene Pakhomov | Owner: nobody
Component: Migrations | Version: 2.0
Severity: Normal | Resolution: fixed
Keywords: default | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e62f6e0968eb19a44198211b5398c5738db454c5" e62f6e09]:
{{{
#!CommitTicketReference repository=""
revision="e62f6e0968eb19a44198211b5398c5738db454c5"
Fixed #29505 -- Removed SchemaEditor's calling of callable defaults.
Thanks Eugene Pakhomov for the suggested fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29505#comment:8>