Code

Opened 2 years ago

Closed 21 months ago

Last modified 21 months ago

#17787 closed Bug (fixed)

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

Reported by: aaugustin Owned by: claudep
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 claudep 22 months ago.
Update override_settings docs

Download all attachments as: .zip

Change History (18)

comment:1 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

comment:2 Changed 2 years ago by aaugustin

  • Owner changed from aaugustin to nobody
  • Severity changed from Release blocker to Normal
  • Triage Stage changed from Design decision needed to Accepted

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 2 years ago by aaugustin

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

comment:4 Changed 2 years ago by aaugustin

In [17597]:

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

comment:5 Changed 2 years ago by aaugustin

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

comment:6 Changed 2 years ago by aaugustin

#17882 has another use case.

comment:7 Changed 2 years ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:8 Changed 2 years ago by aaugustin

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:9 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from reopened to new

comment:10 Changed 2 years ago by aaugustin

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

Last edited 22 months ago by aaugustin (previous) (diff)

comment:11 Changed 2 years ago by ramiro

Last edited 2 years ago by ramiro (previous) (diff)

comment:12 follow-up: Changed 22 months ago by aaugustin

  • Owner changed from aaugustin to nobody
  • Severity changed from Normal to Release 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 22 months ago by claudep

  • 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 22 months ago by claudep

Update override_settings docs

comment:14 Changed 22 months ago by claudep

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

comment:15 Changed 21 months ago by claudep

  • Owner changed from nobody to claudep

comment:16 Changed 21 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In fc2681b22b120a468607c6aeb06163d8e5dbf897:

Fixed #17787 -- Documented reset caches by setting_changed signal

comment:17 Changed 21 months ago by Claude Paroz <claude@…>

In c5da577b9e394b3b4696e4f4ca495c82951b6678:

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

Backport of fc2681b22 from master.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.