[Django] #20348: Consistently handle `Promise` objects assigned to model fields.

5 views
Skip to first unread message

Django

unread,
May 3, 2013, 9:59:26 AM5/3/13
to django-...@googlegroups.com
#20348: Consistently handle `Promise` objects assigned to model fields.
----------------------------------------------+--------------------
Reporter: mrmachine | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Currently, all `Promise` objects are passed to `force_text()` deep in ORM
query code. Not only does this make it difficult or impossible for
developers to prevent or alter this behaviour, but it is also wrong for
non-text fields.

`Field.get_prep_value()` seems like a better place to handle `Promise`
objects, and `_proxy____cast()` seems like a better way to do it than
passing them through `force_text()`. All `Field` subclasses should call
`get_prep_value()` on their super class to ensure they have a real value
to work with.

This change would also facilitate the creation of custom fields like
`PickleField`, which *can* store `Promise` objects, to override this
behaviour.

--
Ticket URL: <https://code.djangoproject.com/ticket/20348>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
May 3, 2013, 10:00:32 AM5/3/13
to django-...@googlegroups.com
#20348: Consistently handle `Promise` objects assigned to model fields.
-------------------------------------+-------------------------------------

Reporter: mrmachine | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by mrmachine):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

Refs #10498.

--
Ticket URL: <https://code.djangoproject.com/ticket/20348#comment:1>

Django

unread,
May 3, 2013, 10:06:04 AM5/3/13
to django-...@googlegroups.com
#20348: Consistently handle `Promise` objects assigned to model fields.
-------------------------------------+-------------------------------------

Reporter: mrmachine | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by mrmachine):

* has_patch: 0 => 1


Comment:

https://github.com/django/django/pull/1040

--
Ticket URL: <https://code.djangoproject.com/ticket/20348#comment:2>

Django

unread,
Jun 16, 2013, 3:53:25 AM6/16/13
to django-...@googlegroups.com
#20348: Consistently handle `Promise` objects assigned to model fields.
-------------------------------------+-------------------------------------

Reporter: mrmachine | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by akaariai):

* stage: Unreviewed => Ready for checkin


Comment:

I like this. This means that lazy values are always evaluated at the time
they are needed by the DB and one can customize the way the promise object
is evaluated per field.

Ill mark this as ready for checkin. The issues keeping me from committing
this straight ahead:
- Is this too risky to commit in alpha stage?
- Could this lead to incompatibilities for custom fields?
- It seems get_prep_lookup() doesn't always use get_prep_value(). I
wonder if this could be fixed...

But, like the "ready for checkin" triage stage indicates I don't see any
of the above as blockers for commit. Waiting a bit to see if any other
opinions arise.

--
Ticket URL: <https://code.djangoproject.com/ticket/20348#comment:3>

Django

unread,
Jul 31, 2013, 8:43:45 AM7/31/13
to django-...@googlegroups.com
#20348: Consistently handle `Promise` objects assigned to model fields.
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: master
(models, ORM) | Resolution: fixed

Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Anssi Kääriäinen <akaariai@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"31e6d58d46894ca35080b4eab7967e4c6aae82d4"]:
{{{
#!CommitTicketReference repository=""
revision="31e6d58d46894ca35080b4eab7967e4c6aae82d4"
Fixed #20348 -- Consistently handle Promise objects in model fields.

All Promise objects were passed to force_text() deep in ORM query code.


Not only does this make it difficult or impossible for developers to
prevent or alter this behaviour, but it is also wrong for non-text
fields.

This commit changes `Field.get_prep_value()` from a no-op to one that
resolved Promise objects. All subclasses now call super() method first
to ensure that they have a real value to work with.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20348#comment:4>

Reply all
Reply to author
Forward
0 new messages