Update to WhiteNoise v2.0.2 and add custom index file & cache max-age support

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
As of bug 1145606, the UI is now being served by gunicorn/WhiteNoise rather than Apache.

In the last hour, a new version of WhiteNoise has been released, which adds some useful features:
https://github.com/evansd/whitenoise/blob/master/CHANGELOG.rst#v20

...notably: The ability to serve files that were not present at the time runserver/gunicorn was run, when in DEBUG mode.

In addition, there are a few other tweaks we can make:
* Since we don't use Django templates, we couldn't use the automatic gzip/caching support provided by whitenoise.django.GzipManifestStaticFilesStorage. As such we need to:
 (a) override the implementation of is_immutable_file() so the built js/css files in dist/ are recognised as immutable, and so get the long Cache-Control max-age set. (All files get a Last-Modified set already, but this would avoid the HTTP 304 round-trip entirely).
 (b) use the WhiteNoise gzip tool as part of the Heroku build, so the .gz equivalent files exist for serving compressed.
* We can switch to using WHITENOISE_ROOT instead of the add_files() call here: https://github.com/mozilla/treeherder/blob/0ea180259eda4f59cde4ceae868a971661bc1621/treeherder/webapp/wsgi.py#L48 (I missed this in the docs the first time around; think I'll submit a PR against them to make it clearer)
* If we wanted, we could make '/' serve '/index.html' directly (so it's not in the URL bar), by sub-classing add_files() (I have a WIP for this locally).
* We can probably remove the staticfiles_urlpatterns() call now that WhiteNoise added the use_finders mode when in DEBUG:
https://github.com/mozilla/treeherder/blob/0ea180259eda4f59cde4ceae868a971661bc1621/treeherder/webapp/urls.py#L31-L34

Related, in later bugs we can:
1) Make the dist directory be build as part of deploy (if we can get nodejs installed on prod; or else wait until we've moved fully to Heroku)
2) Decide if we want to move to serving the main html pages as a Django view, and so can move the rest of the assets to the Django static directory, and get a number of benefits as a result. (I've looked and there does seem to be a django angular templates plugin that would replace what grunt-angular-templates is doing at the moment). Though presumably this would mean breaking the web-server.js workflow (since it wouldn't understand the "{% static "css/foo.css" %}").
(Assignee)

Updated

3 years ago
Blocks: 1176484
(Assignee)

Comment 1

3 years ago
Meant to say: The reason this is important for moving to Heroku, is that we won't be behind Zeus any more, which is what currently provides our gzip compression for prod/stage.
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Created attachment 8630461 [details] [review]
Further changes to gunicorn/WhiteNoise hosted UI
Attachment #8630461 - Flags: review?(mdoglio)
(Assignee)

Updated

3 years ago
Priority: P3 → P2
Summary: Further tweaks to gunicorn/WhiteNoise hosted UI → Update to WhiteNoise v2.0.2 and add custom index file & cache max-age support
Attachment #8630461 - Flags: review?(mdoglio) → review+

Comment 3

3 years ago
Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/6d195ebbc889fe6d1b0190b38ae28e354c6dca24
Bug 1176482 - WhiteNoise: Use WHITENOISE_ROOT instead of add_files()

Instead of manually adding the UI directory using add_files(), we can
use WhiteNoise's WHITENOISE_ROOT variable, per:
http://whitenoise.evans.io/en/latest/django.html?#WHITENOISE_ROOT

https://github.com/mozilla/treeherder/commit/1bd721f7bc0cf5ccd764c2d60dfde4f3404f98b3
Bug 1176482 - WhiteNoise: Update to v2.0.2

https://github.com/evansd/whitenoise/blob/master/CHANGELOG.rst
https://github.com/evansd/whitenoise/compare/v1.0.6...v2.0.2

https://github.com/mozilla/treeherder/commit/9a46df35ce753fc6d53546361d36e842235c080a
Bug 1176482 - WhiteNoise: Remove superfluous staticfiles_urlpatterns()

WhiteNoise 2.0 now uses staticfiles "finders" in development as of:
https://github.com/evansd/whitenoise/commit/25f5757b41f5d876252dff873ecb88b177793a3e
...so we don't need to handle urlpatterns for static asserts in urls.py.

https://github.com/mozilla/treeherder/commit/2748e398ba7346cde8a879b71dd2a214f11dd0cc
Bug 1176482 - WhiteNoise: Combine definitions of urlpatterns

https://github.com/mozilla/treeherder/commit/b28946c9b2d9e8b43d23908540488f6e0d5e4627
Bug 1176482 - WhiteNoise: Serve index.html for bare '/' URIs too

Customise the DjangoWhiteNoise class, so that we add additional dummy
'/' entries to the file mapping, for each index.html static file found.
We also append the index filename to the URL in find_file(), since in
debug mode files are served directly from the filesystem instead of
using the list in `self.files`.

This means we can remove the site root redirect, allowing the Treeherder
homepage to be served from:
https://treeherder-heroku.herokuapp.com/
...rather than:
https://treeherder-heroku.herokuapp.com/index.html

The original add_files() method:
https://github.com/evansd/whitenoise/blob/7de97f915479c46834ba7dcaadbd58e3cdc24b40/whitenoise/base.py#L165-L178

The original find_file() method:
https://github.com/evansd/whitenoise/blob/7de97f915479c46834ba7dcaadbd58e3cdc24b40/whitenoise/django.py#L85-L90

https://github.com/mozilla/treeherder/commit/3a6b45647f7a4947013c7026fad5d00ab0de9613
Bug 1176482 - WhiteNoise: Use infinite cache max-age for dist JS/CSS

The JS/CSS files in the dist directory are given filenames containing
the file SHA by grunt build. As such, these files can be given infinite
cache max-age headers to save the browser experiencing a HTTP 304 round-
trip each time - since when they do change, it will be picked up by the
change of filename in the HTML.

The default DjangoWhiteNoise is_immutable_file() method only works with
Django static files that use the CachedStaticFilesStorage naming scheme:
https://github.com/evansd/whitenoise/blob/7de97f915479c46834ba7dcaadbd58e3cdc24b40/whitenoise/django.py#L92-L110
...whereas grunt-cache-busting uses a scheme like:
index.min-feae259e2c205af67b0e91306f9363fa.js
logviewer.min-7d3b95c2e9323dfe5f825ebb45c7875d.css
(Assignee)

Comment 4

3 years ago
Deployed to Heroku.

The index.html is now no longer required, eg:
https://treeherder-heroku.herokuapp.com/#/jobs?repo=mozilla-inbound

In addition, the assets now have correct max-age headers, eg:

[~/src/treeherder]$ curl -I https://treeherder-heroku.herokuapp.com/css/index.min-bb3491b64c79a99ea3cbc7aa918d564c.css
HTTP/1.1 200 OK
Connection: keep-alive
Server: gunicorn/19.3.0
Date: Wed, 08 Jul 2015 10:26:45 GMT
Last-Modified: Wed, 08 Jul 2015 10:24:11 GMT
Content-Length: 167508
Content-Type: text/css; charset="utf-8"
Cache-Control: public, max-age=315360000
Access-Control-Allow-Origin: *
Via: 1.1 vegur

[~/src/treeherder]$ curl -I https://treeherder-heroku.herokuapp.com/js/index.min-7958499d5a8f0ab4f8b4148e05ca71a6.js
HTTP/1.1 200 OK
Connection: keep-alive
Server: gunicorn/19.3.0
Date: Wed, 08 Jul 2015 10:26:56 GMT
Last-Modified: Wed, 08 Jul 2015 10:24:12 GMT
Content-Length: 763697
Content-Type: application/javascript; charset="utf-8"
Cache-Control: public, max-age=315360000
Access-Control-Allow-Origin: *
Via: 1.1 vegur
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Blocks: 1197796
You need to log in before you can comment on or make changes to this bug.