Add static path precompressed option#791
Add static path precompressed option#791oliverlambson wants to merge 1 commit intoemmett-framework:masterfrom
Conversation
oliverlambson
force-pushed
the
static-files
branch
from
b7da334 to
a9b5a59
Compare
granian/server/common.py
Outdated
| self.static_path = ( | ||
| ( | ||
| static_path_route, | ||
| str(static_path_mount.resolve()), | ||
| (str(static_path_expires) if static_path_expires else None), | ||
| self.static_files = ( | ||
| StaticFilesSettings( | ||
| prefix=static_path_route, | ||
| mount=str(static_path_mount.resolve()), | ||
| expires=str(static_path_expires) if static_path_expires else None, | ||
| precompressed=static_path_precompressed, |
There was a problem hiding this comment.
This change is from original PR. I kept it because the alternative of growing the config tuple grow to include a positional bool feels wrong. It does result in this diff being quite a lot bigger though (it flows through everywhere). I don't think this breaks any public APIs since it's only on the Worker classes, the Server classes just get an extra optional arg
| # only use one interface here since the negotiation logic in Rust is | ||
| # interface-agnostic and tests take long to run | ||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize('server_static_files_precompressed', ['rsgi'], indirect=True) | ||
| @pytest.mark.parametrize( | ||
| 'accept_encoding,expected_encoding', | ||
| [ | ||
| # multiple encodings - client order wins when q-values equal (first one wins) | ||
| ('br, gzip', 'br'), |
There was a problem hiding this comment.
aside: it may be worth optimising test fixture startup time, or if not possible add xdist to run tests in parallel
| headers.insert( | ||
| "cache-control", | ||
| CACHE_CONTROL, | ||
| HeaderValue::from_str(&format!("max-age={expires}")).unwrap(), |
There was a problem hiding this comment.
would be nice to be able to configure public/private and immutable for this header too
|
@oliverlambson thank you for your contribution ❤️ While I originally planned to review this for 2.7, given the amount of changes this introduces, and also the fact that 2.7 is already quite a big set of changes, I rescheduled this for 2.8. Gonna review the code as soon as I can, probably in the following weeks. |
|
@gi0baro that sounds great, thanks! |
oliverlambson
force-pushed
the
static-files
branch
from
a9b5a59 to
180385e
Compare
Adds support for serving pre-compressed static files with .br, .gz, or .zst extensions. When enabled via --static-path-precompressed, the server will: - Check Accept-Encoding header for supported encodings (br, gzip, zstd) - Serve compressed sidecar files if they exist (e.g., file.js.br for file.js) - Respect client q-value priorities for encoding selection - Fall back to uncompressed file if no sidecar exists - Use lazy caching to avoid repeated filesystem checks for sidecars Works with the existing multi-mount and dir-to-file rewrite features.
oliverlambson
force-pushed
the
static-files
branch
from
180385e to
b943109
Compare
|
@gi0baro thanks for 2.7 release, have rebased and updated this branch to suppored the new multi-mount config. Chose to keep the compression option as a global since expires and dir-to-file are both global too |
There was a problem hiding this comment.
Hey @oliverlambson, here's my first pass on this.
Before going into code-specific things, I would:
- drop the caching mechanism entirely. It adds a bunch of code-complexity, and it's not clear at the time being whether its presence is needed or not for performance reasons. I'm up to to reconsider some caching mechanism once the implementation is completed and we actually verify we're not satisfied with the performance we see.
- make the supported encodings an actual parameter configurable by the user. To my perspective it doesn't really makes sense to check for all the possible file formats every time while the user which sets up Granian to serve pre-compressed assets knows which file formats are actually available. With this change, checking for all the formats becomes a worst case scenario which the user voluntarily requested. This probably also allows to refactor the amount of nested for-loop checks in the code and use
HashSetinstead.
2 participants
Closes #577
Carries on the work from #646, changes from that PR:
q=,*)example
./static/styles.css.br- if exists, serve withContent-Encoding: br./static/styles.css.gz- if exists, serve withContent-Encoding: gzip./static/styles.css- fallback to uncompressedchanges to public apis
granian app:app \ --static-path-mount ./static \ --static-path-route /static \ --static-path-expires 31536000 \ --static-path-precompressed # new (default disabled)