Skip to content

Conversation

prandla
Copy link
Member

@prandla prandla commented Aug 24, 2025

This makes it unnecessary to do a hard refresh after editing static files (especially relevant for cws_utils.js or cws_style.css), as the edited files will have a different hash and thus will be cached separately.

I actually would have liked to do this for RWS too, but RWS's Ranking.html is a static file, not a jinja template. I guess we'd need some different solution for it.

Copy link
Contributor

@gollux gollux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like adding hashes to URLs of static files, but I wonder how much does calculation of hashes on the fly slow down the first few requests.

Wouldn't it be better to calculate hashes of all static files when the service starts?

Also, I wonder if we can safely assume that each static file can be loaded into memory. Maybe we could use file_digest() instead?

@prandla
Copy link
Member Author

prandla commented Aug 24, 2025

I like adding hashes to URLs of static files, but I wonder how much does calculation of hashes on the fly slow down the first few requests.

I didn't really notice it. The files aren't that big and sha256 is not that slow.

I went and measured it. Before this PR, the first load took (on average) around 380ms. Now, it's around 405ms. (Subsequent loads are around 40ms in both cases). So, clearly this isn't the worst thing that happens on first load :)

(i guess it's jinja compiling its templates? dunno what else it could be.)

Wouldn't it be better to calculate hashes of all static files when the service starts?

I think that's a bit annoying to implement; importlib.resources doesn't provide a .walk function so it's annoying to get a list of all files in the static folder.

Also, I wonder if we can safely assume that each static file can be loaded into memory. Maybe we could use file_digest() instead?

I'm pretty sure Tornado buffers the entire response anyways (otherwise we wouldn't need FileServerMiddleware), so the file already needs to fit into memory.

Also, I don't see a function called file_digest anywhere, though I remember needing one at some point... maybe it was in a scrapped PR.
edit: never mind, found it, it was hashlib.file_digest (i thought you meant something from cmscommon/digest.py). yeah, using that sounds good.

@prandla prandla force-pushed the cws-cache-busting branch from 397d156 to df254a3 Compare August 25, 2025 02:01
This makes it unnecessary to do a hard refresh after editing static
files (especially relevant for cws_utils.js or cws_style.css), as the
edited files will have a different hash and thus will be cached
separately.
@prandla prandla force-pushed the cws-cache-busting branch from df254a3 to 831d6f0 Compare August 31, 2025 08:18
@prandla prandla requested a review from veluca93 August 31, 2025 08:19
prandla added a commit to e-i-o/cms that referenced this pull request Sep 26, 2025
…ream PR cms-dev#1524)

This makes it unnecessary to do a hard refresh after editing static
files (especially relevant for cws_utils.js or cws_style.css), as the
edited files will have a different hash and thus will be cached
separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants