Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Avoid caching of config file #496

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

frankkopp
Copy link
Member

Summary of Changes

As it sometimes takes forever for users to see new configs we should make sure the config file is not cached.

I tried with a http header which didn't work. So added a cache-killer parameter.

Additional context

https://discord.com/channels/738864299392630914/846470774361161808/1307387181605847120

Discord username (if different from GitHub): cdr_maverick

@frankkopp frankkopp self-assigned this Nov 16, 2024
@Revyn112
Copy link
Contributor

Revyn112 commented Nov 16, 2024

@frankkopp How you've tried to set the header?
Just wondering, as we are using the cache control within the fetch options and have no problems with cached configuration files so far.

return await fetch(url, { cache: "no-store" })

https://github.com/headwindsim/installer/blob/a2087477e5391dd2c33b90d453e3ef95fda31d1f/src/renderer/utils/InstallerConfiguration.ts#L406

@pdellaert
Copy link

pdellaert commented Nov 16, 2024

If the cache: "no-store" works, I'd prefer that, as it is a cleaner approach. But not sure how aggressive Cloudflare caching can be.

@frankkopp
Copy link
Member Author

@frankkopp How you've tried to set the header?

Just wondering, as we are using the cache control within the fetch options and have no problems with cached configuration files so far.

return await fetch(url, { cache: "no-store" })

https://github.com/headwindsim/installer/blob/a2087477e5391dd2c33b90d453e3ef95fda31d1f/src/renderer/utils/InstallerConfiguration.ts#L406

Can try that. But I'm rather sure it had this in it as well.

@FoxtrotSierra6829
Copy link
Member

If the cache: "no-store" works, I'd prefer that, as it is a cleaner approach. But not sure how aggressive Cloudflare caching can be.

The issue is not about the caching in Cloudflare's CDN, but the Installer's dynamic Chromium disk cache.

@frankkopp
Copy link
Member Author

I tested a few more headers - but I think they do not influence the internal cache of the Chromium cache.
Only the cache-killer seem to work.

return await fetch(url, { cache: 'no-store' })

or these:

return await fetch(url, { headers: { 'Cache-Control': 'no-store, no-cache, must-revalidate, max-age=0, private' } })

The server (CloudFlare) seems to also override these headers in its response:
image

So - it seems only the cache-killer param works.

@pdellaert
Copy link

pdellaert commented Nov 16, 2024

In that case, cache-killer is the way forward :). Thanks for all the testing!

@frankkopp frankkopp merged commit 022c0b6 into flybywiresim:master Nov 17, 2024
3 checks passed
@frankkopp frankkopp deleted the config-cache-killer branch November 17, 2024 09:48
frankkopp added a commit that referenced this pull request Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants