[Django] #23381: Context manager should determine their initial state at __enter__, not __init__

5 views
Skip to first unread message

Django

unread,
Aug 28, 2014, 4:28:49 PM8/28/14
to django-...@googlegroups.com
#23381: Context manager should determine their initial state at __enter__, not
__init__
-------------------------------+--------------------
Reporter: tchaumeny | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Some context manager use __init__ to determine their initial state and
__exit__ to restore that state. The correct way to use context manager is
to determine the initial state within the __enter__ method. This is
especially dangerous when using `ContextDecorator` where initialization
time can be different from "entering" time.

Flawed context managers are `django.utils.translation.override` (which
leads to a bug following the use of `ContextDecorator`) and
`django.utils.timezone.override`. The pull request below fix them both and
updates a test to demonstrate the failure with
`django.utils.translation.override`.

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

Django

unread,
Aug 28, 2014, 4:32:17 PM8/28/14
to django-...@googlegroups.com
#23381: Context manager should determine their initial state at __enter__, not
__init__
-------------------------------+--------------------------------------

Reporter: tchaumeny | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

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


Comment:

Here is the pull request https://github.com/django/django/pull/3135

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

Django

unread,
Aug 28, 2014, 4:49:02 PM8/28/14
to django-...@googlegroups.com
#23381: Context manager should determine their initial state at __enter__, not
__init__
---------------------------+------------------------------------
Reporter: tchaumeny | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

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

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

* component: Uncategorized => Utilities
* needs_better_patch: 0 => 1
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted


Comment:

Good catch. See my comments on the pull request. If you're not sure how to
write the tests, let me know, I can take care of that part.

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

Django

unread,
Aug 28, 2014, 5:15:30 PM8/28/14
to django-...@googlegroups.com
#23381: Context manager should determine their initial state at __enter__, not
__init__
---------------------------+---------------------------------------------
Reporter: tchaumeny | Owner: nobody

Type: Bug | Status: new
Component: Utilities | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin

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

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

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin


Comment:

Updated PR looks good.

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

Django

unread,
Aug 28, 2014, 7:22:57 PM8/28/14
to django-...@googlegroups.com
#23381: Context manager should determine their initial state at __enter__, not
__init__
---------------------------+---------------------------------------------
Reporter: tchaumeny | Owner: nobody
Type: Bug | Status: closed
Component: Utilities | Version: 1.6
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------+---------------------------------------------
Changes (by Simon Charette <charette.s@…>):

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


Comment:

In [changeset:"efcbf3e095dce3491eb52774978afe3f7bbdf217"]:
{{{
#!CommitTicketReference repository=""
revision="efcbf3e095dce3491eb52774978afe3f7bbdf217"
Fixed #23381 -- Context manager restored state should be determined in
__enter__
}}}

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

Reply all
Reply to author
Forward
0 new messages