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

FileFetcher/HttpCache should respect cache control headers #9931

Closed
kitsonk opened this issue Mar 30, 2021 · 4 comments · Fixed by #13010
Closed

FileFetcher/HttpCache should respect cache control headers #9931

kitsonk opened this issue Mar 30, 2021 · 4 comments · Fixed by #13010
Assignees
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Mar 30, 2021

Current with the Deno dependency cache, we currently don't support cache control headers. Basically once a module is cached, it is never reloaded until it is deleted, unless the --reload flag is specified. At that point, the cached ETag is sent back for efficiency.

While this may not be a huge issue in some cases, some time-sensitive modules and other information will never be invalidated, even though the server is giving clear guidance to how long to persist the data without checking back. For example a "latest" release of a module or information about registry completions which the Deno language server persists won't ever get updated through "normal use".

We have sufficient controls in place to ensure that people can "control" what is in their cache, including package locks and cache only flags, so it makes sense that we should be more "browser like" and respect the cache headers and allow the hoster to give guidance about the remote modules.

@kitsonk kitsonk added cli related to cli/ dir suggestion suggestions for new features (yet to be agreed) labels Mar 30, 2021
@lucacasonato
Copy link
Member

I think we have to make a distinction between cached user code, and registry manifests / completions here. Registry manifests and registry completions should respect the cache control headers (and only those). I think however that user code should not. User code should be indefinitely cached, like it is now, and only reloaded when specifying the --reload flag. Changing this behaviour would be a huge breaking change.

I think the registry completions and manifests should be cached separately from the user code. We store user code in the deps and gen folders. We should store registry completions separately in a registry folder IMO.

@kitsonk
Copy link
Contributor Author

kitsonk commented Mar 30, 2021

@lucacasonato that is already being done (the seperate path)...

@wperron
Copy link
Contributor

wperron commented Jun 11, 2021

Now that we have SQLite in the dependencies, I wonder if we could be able to leverage it make an efficient index of the cached files and their cache headers?

@kitsonk
Copy link
Contributor Author

kitsonk commented Nov 9, 2021

We now cache an empty document when the configuration is not available for an origin. We should also write out a expiration header that is +24 hours, so that we try a maximum of once every 24 hours to check the host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants