Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#17787 closed Bug (fixed)

Clear setting-dependant caches when settings are overridden (in tests)

Reported by: Aymeric Augustin Owned by: Claude Paroz
Component: Testing framework Version: 1.4-beta-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

r16237 introduced a setting_changed signal, triggered by the override_settings decorator / context manager.

However, Django doesn't use this signal to reset caches that depend on built-in settings. In the current state of thing, the signal is only usable is third-party code.

Is this a design choice or a bug?

Marking as release blocker since it's a major bug in a new feature.

Attachments (1)

17787-docs.diff (1.7 KB ) - added by Claude Paroz 12 years ago.
Update override_settings docs

Download all attachments as: .zip

Change History (18)

comment:1 by Aymeric Augustin, 13 years ago

Owner: changed from nobody to Aymeric Augustin

comment:2 by Aymeric Augustin, 13 years ago

Owner: changed from Aymeric Augustin to nobody
Severity: Release blockerNormal
Triage Stage: Design decision neededAccepted

We discussed this on IRC and determined that:

  • this should be fixed eventually — override_settings doesn't reach its full potential until then
  • this doesn't need to be fixed in 1.4 — override_settings is useful, even without this, and we don't want to make too intrusive changes at this point

comment:3 by Aymeric Augustin, 13 years ago

It would be nice to add a note mentioning this limitation in the 1.4 docs.

comment:4 by Aymeric Augustin, 13 years ago

In [17597]:

Clarified the fact that the signal_changed signal isn't used by Django itself (yet). Refs #17787.

comment:5 by Aymeric Augustin, 13 years ago

#17744 is an instance of this problem, with MEDIA_ROOT.

comment:6 by Aymeric Augustin, 13 years ago

#17882 has another use case.

comment:7 by Claude Paroz, 13 years ago

Resolution: fixed
Status: newclosed

Several settings are now using the setting_changed signal. I don't think it's useful to get this one open.

comment:8 by Aymeric Augustin, 13 years ago

Resolution: fixed
Status: closedreopened

I'd like to review a few more settings :)

comment:9 by Aymeric Augustin, 13 years ago

Owner: changed from nobody to Aymeric Augustin
Status: reopenednew

comment:10 by Aymeric Augustin, 13 years ago

Also [2ddfcfbec6] should be reverted before closing this ticket.

Last edited 12 years ago by Aymeric Augustin (previous) (diff)

comment:11 by Ramiro Morales, 12 years ago

Last edited 12 years ago by Ramiro Morales (previous) (diff)

comment:12 by Aymeric Augustin, 12 years ago

Owner: changed from Aymeric Augustin to nobody
Severity: NormalRelease blocker

We're in an inconsistent state right now. Some settings are properly reset and others aren't; the docs still say that the signal isn't used.

I don't think I'm going to work on this exhaustively.

We still have to document the new behavior before the next release. At worst, open django/tests/signals.py, make a list of settings that are reset, and put that in the docs...

in reply to:  12 comment:13 by Claude Paroz, 12 years ago

Has patch: set

Replying to aaugustin:

We're in an inconsistent state right now. Some settings are properly reset and others aren't;

Let's create tickets for each setting unproperly reset. But auditing the code to relate settings with global cached variables might be something hard. I'm afraid only bugs will reveal those.

the docs still say that the signal isn't used.

We still have to document the new behavior before the next release. At worst, open django/tests/signals.py, make a list of settings that are reset, and put that in the docs...

I'm attaching a patch for updating the docs.

by Claude Paroz, 12 years ago

Attachment: 17787-docs.diff added

Update override_settings docs

comment:14 by Claude Paroz, 12 years ago

I just committed one more in c7f44ae085df3a270aa998cdedb56f36900cb9ef (TEMPLATE_LOADERS). Let's not forget it when committing.

comment:15 by Claude Paroz, 12 years ago

Owner: changed from nobody to Claude Paroz

comment:16 by Claude Paroz <claude@…>, 12 years ago

Resolution: fixed
Status: newclosed

In fc2681b22b120a468607c6aeb06163d8e5dbf897:

Fixed #17787 -- Documented reset caches by setting_changed signal

comment:17 by Claude Paroz <claude@…>, 12 years ago

In c5da577b9e394b3b4696e4f4ca495c82951b6678:

[1.5.x] Fixed #17787 -- Documented reset caches by setting_changed signal

Backport of fc2681b22 from master.

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