Opened 8 years ago

Closed 8 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 8 years ago.
Patch
sites2.diff (1.3 KB) - added by Boo 8 years ago.
Patch v.2
sites_v3.diff (1022 bytes) - added by Gert Van Gool 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by Boo

Attachment: sites.diff added

Patch

comment:1 Changed 8 years ago by Malcolm Tredinnick

Needs tests: set
Patch needs improvement: set
Summary: Simple patch for contrib.sitesKeep SITE_CACHE cache in line with changes to Site instances
Triage Stage: UnreviewedAccepted

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 8 years ago by Boo

Attachment: sites2.diff added

Patch v.2

comment:2 in reply to:  1 Changed 8 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 8 years ago by Boo

Triage Stage: AcceptedReady for checkin

comment:4 Changed 8 years ago by Malcolm Tredinnick

Needs tests: unset
Triage Stage: Ready for checkinAccepted

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 8 years ago by Gert Van Gool

Attachment: sites_v3.diff added

comment:5 in reply to:  4 Changed 8 years ago by Gert Van Gool

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 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 Changed 8 years ago by Marc Fargas

Resolution: fixed
Status: newclosed

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

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