Opened 10 years ago

Closed 10 years ago

#23381 closed Bug (fixed)

Context manager should determine their initial state at __enter__, not __init__

Reported by: Thomas C Owned by: nobody
Component: Utilities Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (4)

comment:2 by Aymeric Augustin, 10 years ago

Component: Uncategorized → Utilities
Patch needs improvement: set
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Bug

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.

comment:3 by Aymeric Augustin, 10 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

Updated PR looks good.

comment:4 by Simon Charette <charette.s@…>, 10 years ago

Resolution: → fixed
Status: new → closed

In efcbf3e095dce3491eb52774978afe3f7bbdf217:

Fixed #23381 -- Context manager restored state should be determined in enter

Note: See TracTickets for help on using tickets.
Back to Top