Code

Opened 6 years ago

Closed 5 years ago

#9228 closed (fixed)

Keep SITE_CACHE cache in line with changes to Site instances

Reported by: Boo Owned by: nobody
Component: Contrib apps Version: master
Severity: Keywords:
Cc: boobsd@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

This patch updates SITE_CACHE after saving Site model.
You do not have to restart Django-project after changing Site model.

Attachments (3)

sites.diff (448 bytes) - added by Boo 6 years ago.
Patch
sites2.diff (1.3 KB) - added by Boo 6 years ago.
Patch v.2
sites_v3.diff (1022 bytes) - added by gvangool 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by Boo

Patch

comment:1 follow-up: Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement set
  • Summary changed from Simple patch for contrib.sites to Keep SITE_CACHE cache in line with changes to Site instances
  • Triage Stage changed from Unreviewed to Accepted

Changed the title to describe the problem being addressed.

This is a good idea, but the implementation needs some work before it can go in:

  • It needs a test. There is a tests.py in django/contrib/sites/.
  • The save() method takes a couple of required parameters, not just self (see the Model documentation).
  • To avoid duplicating work, in case we change how things are loaded into that cache, it will be better to just remove the relevant key from the SITE_CACHE dictionary. Then the next call to get_current() will reload it correctly (whatever "correctly" means now or in the future).

Changed 6 years ago by Boo

Patch v.2

comment:2 in reply to: ↑ 1 Changed 6 years ago by Boo

Replying to mtredinnick:

This is a good idea, but the implementation needs some work before it can go in:

  • It needs a test. There is a tests.py in django/contrib/sites/.
  • The save() method takes a couple of required parameters, not just self (see the Model documentation).
  • To avoid duplicating work, in case we change how things are loaded into that cache, it will be better to just remove the relevant key from the SITE_CACHE dictionary. Then the next call to get_current() will reload it correctly (whatever "correctly" means now or in the future).

Ok, done.

comment:3 Changed 6 years ago by Boo

  • Triage Stage changed from Accepted to Ready for checkin

comment:4 follow-up: Changed 6 years ago by mtredinnick

  • Needs tests unset
  • Triage Stage changed from Ready for checkin to Accepted

I applied the test part of the patch and left out the other part, ran the tests and they passed. A test for a fix should fail before the fix is applied and pass afterwards and this test isn't doing that.

Also, you patch mixes tabs and spaces. It should be spaces only for indentation, please.

Changed 5 years ago by gvangool

comment:5 in reply to: ↑ 4 Changed 5 years ago by gvangool

Replying to mtredinnick:

I applied the test part of the patch and left out the other part, ran the tests and they passed. A test for a fix should fail before the fix is applied and pass afterwards and this test isn't doing that.

Also, you patch mixes tabs and spaces. It should be spaces only for indentation, please.

I ran into the same thing. As far as I can see, it has something to do with the difference between running in the shell and Apache2.

comment:6 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:7 Changed 5 years ago by telenieko

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

This was implemented in [9908] and [9909] (removing the item from the cache, so it gets refreshed later).

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.