If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Etags ignore autoland status output

RESOLVED FIXED

Status

MozReview
General
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dminor, Assigned: smacleod)

Tracking

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Right now to see autoland results a hard refresh (shift+F5) is required. Some suggestions from the MozReview meeting on Oct. 29:
* inject javascript to grab and render results atop a static page
* disable etags [ https://docs.djangoproject.com/en/1.8/ref/settings/#std:setting-USE_ETAGS ]

I think we need some kind of workaround prior to enabling autoland to inbound.
(Reporter)

Comment 1

2 years ago
It looks like things have improved here, a normal refresh picks up the updated status.
(Assignee)

Comment 2

2 years ago
(In reply to Dan Minor [:dminor] from comment #1)
> It looks like things have improved here, a normal refresh picks up the
> updated status.

A normal refresh will pick up the status change if something that affects the etag has changed as well (review has been posted, new diff, etc) - are you sure this isn't what you saw?
(Assignee)

Updated

2 years ago
Priority: -- → P1
(Assignee)

Updated

2 years ago
Assignee: nobody → smacleod
Status: NEW → ASSIGNED
(Assignee)

Comment 3

2 years ago
Created attachment 8693024 [details]
MozReview Request: mozreview: prevent RB from generating etags for review request pages (Bug 1221989). r?mdoglio

mozreview: prevent RB from generating etags for review request pages (Bug 1221989). r?mdoglio

Review Board does not allow custom fields or extensions to effect etag
generation - Any server side modifications to what an extension would
display will be stale in a browser using caching.

To workaround this we remove etags Review Board would send to the client
as well as the HTTP_IF_NONE_MATCH header sent by the client (for a
select set of views we care about).

This will result in a bit of a performance hit but it shouldn't be too
noticeable as the really heavyweight requests like diff fragments (which
are fetched asynchronously from the diff viewer) will still be cached.
Attachment #8693024 - Flags: review?(mdoglio)
Comment on attachment 8693024 [details]
MozReview Request: mozreview: prevent RB from generating etags for review request pages (Bug 1221989). r?mdoglio

https://reviewboard.mozilla.org/r/26379/#review23889

::: pylib/mozreview/mozreview/tests/test-browser-cache.py:64
(Diff revision 1)
> +        time.sleep(1) # Give time for the comment box to become focused

Why do you need to wait for the focus instead of the element presence? Once it's there you should be able to call send_keys on it.
Attachment #8693024 - Flags: review?(mdoglio) → review+
(Assignee)

Comment 5

2 years ago
https://reviewboard.mozilla.org/r/26379/#review23889

> Why do you need to wait for the focus instead of the element presence? Once it's there you should be able to call send_keys on it.

I was having a lot of problems with making that work - I think it has something to do with codemirror being weird or something. I'd prefer to not spend a lot more time trying to fix this atm.
(Assignee)

Comment 6

2 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/cc7ec937c459
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.