582103 - The API's {% cache %} needs to know about api_version

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: API, defect)

Description

[Wil Clouser [:clouserw]]

We're getting is caching big chunks of template code with "{% cache addon %}" but inside that block are if statements which change the output depending on api_version.  We need to tell the cache which api_version it's looking at so we don't get weird results.

Comment 1

[Wil Clouser [:clouserw]]

> We're getting is caching big chunks 

move along

Comment 2

[Wil Clouser [:clouserw]]

davedash is talking about adding context to {% cache %} so this stuff doesn't happen in the future.  I don't know how big of a job that is, but if it's big, we should make it a new bug.

Can this bug be fixed by putting the add-on and the api-version in a list before passing it to cache()?

Comment 3

[Dave Townsend [:mossop]]

This block bug 551274 which is currently a beta4 blocker. How does that sit with your release schedule?

Comment 4

[Wil Clouser [:clouserw]]

Beta 4 isn't on https://wiki.mozilla.org/Releases so I don't know what that schedule is.  What is the freeze date for it?

Comment 5

[Dave Townsend [:mossop]]

(In reply to comment #4)
> Beta 4 isn't on https://wiki.mozilla.org/Releases so I don't know what that
> schedule is.  What is the freeze date for it?

I would anticipate a release somewhere around 20th August (https://wiki.mozilla.org/Firefox/4/Beta)

Comment 6

[Wil Clouser [:clouserw]]

(In reply to comment #5)
> (In reply to comment #4)
> > Beta 4 isn't on https://wiki.mozilla.org/Releases so I don't know what that
> > schedule is.  What is the freeze date for it?
> 
> I would anticipate a release somewhere around 20th August
> (https://wiki.mozilla.org/Firefox/4/Beta)

We'll be good

Comment 7

[Jeff Balogh (:jbalogh)]

(In reply to comment #2)
> davedash is talking about adding context to {% cache %} so this stuff doesn't
> happen in the future.  I don't know how big of a job that is, but if it's big,
> we should make it a new bug.

The only context it takes into account is request.APP.  There isn't a way to add other variants to the cache string right now.

> Can this bug be fixed by putting the add-on and the api-version in a list
> before passing it to cache()?

No.

Comment 8

[Jeff Balogh (:jbalogh)]

r? davedash: 
http://github.com/jbalogh/django-cache-machine/compare/extra-cache-tag
http://github.com/jbalogh/zamboni/compare/extra-cache-tag

Comment 9

[Jeff Balogh (:jbalogh)]

http://github.com/jbalogh/zamboni/commit/1d8b98c
redis: http://github.com/jbalogh/django-cache-machine/commit/89ce18eb8fa8486fc471f03b8c7282c37195de16
master: http://github.com/jbalogh/django-cache-machine/commit/44a3b07c39eaad61b1b485c227ec999c83c9233b

Comment 10

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

Actually this is qa'able... albeit difficult

You should be able to oscillate between 1.2 and 1.5 API and verify that they are indeed different and behaving according to their individual specs.

The problem was we'd cache pages with a key based on the template filename alone, and not on the URL, so since 1.5 and 1.2 used the same template file... we'd get a cache namespace collision.