launchpad-reviewers team mailing list archive

[Colin Watson <mp+444125@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-assets-favicon into launchpad:master.

Commit message:
charm: Serve favicons from assets charm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/444125

This allows us to serve these files from the frontends without hitting the appservers.  I'm not sure why we serve `lib/canonical/launchpad/images/launchpad.png` here rather than `lib/canonical/launchpad/images/favicon.ico` (and regardless of file extension, too!), but this is what we currently do on production so I thought it best to start by matching the existing setup.

I think in practice most browsers will prefer the icons declared in HTML via `lib/lp/app/templates/base-layout.pt`, but it doesn't hurt to serve these files too.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-assets-favicon into launchpad:master.

diff --git a/charm/launchpad-assets/templates/vhost.conf.j2 b/charm/launchpad-assets/templates/vhost.conf.j2
index 65bc07f..b16fabb 100644
--- a/charm/launchpad-assets/templates/vhost.conf.j2
+++ b/charm/launchpad-assets/templates/vhost.conf.j2
@@ -33,5 +33,11 @@
         Require all granted
     </Location>
     Alias "/@@/" "{{ code_dir }}/lib/canonical/launchpad/images/"
+
+    <LocationMatch "^/favicon\.(?:ico|gif|png)$">
+        Header set Cache-Control "public,max-age=5184000"
+        Require all granted
+    </LocationMatch>
+    AliasMatch "^/favicon\.(?:ico|gif|png)$" "/srv/launchpad/code/lib/canonical/launchpad/images/launchpad.png"
 </VirtualHost>