Merge lp://staging/~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions into lp://staging/~abentley/charms/trusty/apache2-reverseproxy/apache-website-interface

Proposed by Martin Packman
Status: Merged
Merged at revision: 22
Proposed branch: lp://staging/~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions
Merge into: lp://staging/~abentley/charms/trusty/apache2-reverseproxy/apache-website-interface
Diff against target: 130 lines (+52/-6)
3 files modified
README.md (+8/-0)
config.yaml (+4/-0)
scripts/config-changed (+40/-6)
To merge this branch: bzr merge lp://staging/~gz/charms/trusty/apache2-reverseproxy/gzip_encoded_extensions
Reviewer Review Type Date Requested Status
Aaron Bentley Approve
Review via email: mp+261139@code.staging.launchpad.net

Description of the change

Adds a new configuration option to serve gzip encoded files with the correct content-encoding header

Two main things of interest:

* We should arguably set the content encoding in S3. This branch does not preclude doing that as well. Unfortunately s3cmd doesn't make it easy, and we'd have to fix up the previous files. See:

<https://github.com/s3tools/s3cmd/issues/396>

Which also raises the other side, if we have Content-Encoding set in S3, we really want the reverse proxy to *always* set accept gzip, and handle the vary/not vary stuff locally - rather than passing through to S3 and letting them decode and transfer uncompressed data.

* Rather than juse using mod_mime AddEncoding, this branch takes a more convoluted approach so it's possible to limit on multiple extensions. This should address Aaron's previous concern as we can then configure just ".log.gz" to be marked, we we know is safe to be displayed in browser.

In the LocationMatch block, we're first disabling mod_deflate (which otherwise futzes with things), then just setting the Content-Encoding header. No Vary business going on, we'd prefer that browsers and other tools get the encoded version.

It would be nice to add some unit tests, we can maybe refactor a bit later?

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

Please add docs to the README. See also inline comments.

review: Needs Fixing
23. By Martin Packman

Address review by abentley

Revision history for this message
Martin Packman (gz) wrote :

Pushed up a change with the things you mentioned.

Revision history for this message
Aaron Bentley (abentley) wrote :

One small change requested inline.

review: Approve
24. By Martin Packman

Use custom InvalidConfig as suggested by abentley in review

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: