The signoff2 view cashes too good, I often see the same pending sign-offs after I accept one. We may want to use the latest action as key to a ETag?

What are the steps to reproduce it?

You'll need to sign off and/or review. Can you get an updated snapshot on -dev- ? Then I can grant you priviledges on that version.

Stealing this bug from Peter. I had to cache not just on latest action, but also on the permissions we use as otherwise the page doesn't update the UX. That is, the vary on cookie apparently get's overruled by the ETag. Given that this is real webdev, I'd like to get Peter's review on this. Also, stas for business logic here.

Comment on attachment 535339 [details] [diff] [review] etag on permissions and latest action Review of attachment 535339 [details] [diff] [review]: ----------------------------------------------------------------- ::: apps/shipping/views/signoff.py @@ +104,5 @@ > > + > +def etag_signoff(request, locale_code, app_code): > + actions = Action.objects.filter(signoff__locale__code=locale_code, > + signoff__appversion__code=app_code).order_by('-pk') How about: try: _id = Action.objects.filter(...).latest('when').id except DoesNotExist: _id = "no signoff" You could also add class Meta: get_latest_by = 'when' to shipping.Action, and then use latest() without any args.

Comment on attachment 535339 [details] [diff] [review] etag on permissions and latest action Review of attachment 535339 [details] [diff] [review]: ----------------------------------------------------------------- ::: apps/shipping/views/signoff.py @@ +109,5 @@ > + can_signoff = request.user.has_perm('shipping.add_signoff') > + review_signoff = request.user.has_perm('shipping.review_signoff') > + try: > + _id = str(actions.values_list('id',flat=True)[0]) > + except: Why a catch-all? I'd suspect the only acceptable exception we can expect is IndexError. So it should be: except IndexError:

https://github.com/mozilla/elmo/commit/32d19258fbe3f216bb4818bc80b1aab386ebc88b, FIXED. Technically and network wise, this was done as a hotfix. I fail to deploy on -dev-, though, mostly because I can't recover from the env change without stas' brain. As that deployment is failing, I'm not deploying on -stage- either. So it's not all that hot.

This doesn't really work in practice. Shyam, any idea why we send out ETags on bm-l10n-dashboard01 that don't make it to the client? Example URL would be https://l10n-stage-sj.mozilla.org/shipping/signoffs/is/fx-aurora. Once I reload it's updated, but just surfing to the page get's an old ETag response.

I see "X-Cache-Info" headers in the responses, that sounds like zeus is talking to us, http://www.zeus.com/community/articles/making-most-content-caching. I verified locally that on a revisit to a signoff page, we're not getting a hit to confirm the etag. Probably we need to make Cache-Control: no-cache ? Stabbing in the dark here, http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9 doesn't make me really understand what we should do. In particular the cache verification stuff with max-age:0. Zeus says "no-cache", and the http docs read like it's the right way to enforce revalidation every time.

working on an update here.

This should fix Zeus caching our responses, and also fixes a few missing datapoints on the etag. Also, with comments now. I only checked the fragment I touched for pep-iness, the rest of the file is still dirty as harry.

Comment on attachment 549079 [details] [diff] [review] make cache-control private, and add pushes and runs to etag Review of attachment 549079 [details] [diff] [review]: -----------------------------------------------------------------

https://github.com/mozilla/elmo/commit/48d548ab4f8482bc0e1fd7732b775955bb143273, marking FIXED again. I updated -dev- and verified that it does what I expect, at least while reviewing a sign-off, which does reload, and when going the roundtrip without a mod, which get's the not-modified.