Opened 5 years ago

Closed 4 years ago

Last modified 4 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 4 years ago.
Update override_settings docs

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin

comment:2 Changed 5 years ago by Aymeric Augustin

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 Changed 5 years ago by Aymeric Augustin

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

comment:4 Changed 5 years ago by Aymeric Augustin

In [17597]:

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

comment:5 Changed 5 years ago by Aymeric Augustin

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

comment:6 Changed 5 years ago by Aymeric Augustin

#17882 has another use case.

comment:7 Changed 5 years ago by Claude Paroz

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 Changed 5 years ago by Aymeric Augustin

Resolution: fixed
Status: closedreopened

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

comment:9 Changed 5 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin
Status: reopenednew

comment:10 Changed 5 years ago by Aymeric Augustin

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

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

comment:11 Changed 4 years ago by Ramiro Morales

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

comment:12 Changed 4 years ago by Aymeric Augustin

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...

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

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.

Changed 4 years ago by Claude Paroz

Attachment: 17787-docs.diff added

Update override_settings docs

comment:14 Changed 4 years ago by Claude Paroz

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

comment:15 Changed 4 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz

comment:16 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In fc2681b22b120a468607c6aeb06163d8e5dbf897:

Fixed #17787 -- Documented reset caches by setting_changed signal

comment:17 Changed 4 years ago by Claude Paroz <claude@…>

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