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

Url Mime Caching #743

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Url Mime Caching #743

wants to merge 9 commits into from

Conversation

kernelkind
Copy link
Member

If the mime type hosted at a URL can't be inferred from the URL, a HEAD request is made and the result is cached. Every so often the data structure is saved to disk

Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
Signed-off-by: kernelkind <[email protected]>
@kernelkind kernelkind requested a review from jb55 February 21, 2025 16:47
@jb55
Copy link
Contributor

jb55 commented Feb 21, 2025

Review for "Url Mime Caching" #743
by kernelkind

Here's a brief review of your PR:

Handling the HEAD Response: There's a subtlety in ehttp_get_mime_type. When the server returns an error or fails to include a Content-Type header, the promise is
never completed (sender.send(...) is never invoked in the error branch). Because of that:

• The in_flight entry for that URL remains in the HashMap forever.
• "UrlMimes::get" will keep returning None for that URL indefinitely, since the promise never resolves or fails.
• Future attempts to retrieve a MIME type for that URL won't spawn another HEAD request, effectively leaving it stuck.

You likely want to call sender.send(...) in both success and failure paths so that the promise always completes, or otherwise remove the promise from the in_flight
Map.

@jb55
Copy link
Contributor

jb55 commented Feb 21, 2025

shouldn't this be a part of imagecache? do we really want to prop drill this independently?

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.

2 participants