-
Notifications
You must be signed in to change notification settings - Fork 219
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
Reevaluate thumbnail fallback behavior #510
Comments
@zackkrida do you know if this happens in practice? Theoretically dead link filtering should prevent this. Of course, there's always the possibility of a mismatch between the HEAD request used by dead link filtering and the real request used by the thumbnail proxy. We could add a Plausible event to see how often this happens. I'd be curious to see whether there are real cases where this is a problem, rather than just hypothetically. If it is, it might suggest an issue with dead link detection, rather than something the frontend needs to consider... but it might be both. |
@sarayourfriend good questions, thanks for revisiting this.
I think this assumption is pretty inaccurate in hindsight. I at least have no evidence to support it. The titular issue to "Reevaluate thumbnail fallback behavior" can be closed, IMO, as we haven't encountered this problem in a long time. If images are unusually large to the point that thumbnails can not be generated or requests time out, I think we should fix that in the provider scripts themselves and see if a more usable file is available for the api's As I recall this issue was written in response to some of the SMK images being multiple GB in size, and we fixed it with the DAG-based approach I shared above.
If we chose to investigate this, do you think we could look at API logs and thumbnail proxy error codes and avoid the need for a plausible event? |
Only if we think API logs will accurately reflect user experience to a meaningful degree. There's always the chance of something going wrong after the API has responded and clients not being able to receive the response for one reason or another. That's only known to the client. I think it's a fine starting point to look at the API response logs to see how much (if any) mismatch there is, but it's important to understand the limitations of that approach, depending on which perspective we're trying to capture. We have an issue elsewhere to incorporate thumbnail errors into the dead link cache, IIRC, which might imply there's at least some assumption of misalignment between how we filter dead links and which thumbnail requests happen. Someone can of course pass |
@zackkrida #4665 has me thinking that it would be good to prioritise this. We show that "image not available" thing, so in some ways this general "thumbnail fallback" question is resolved (I am still on holiday so haven't looked into the history of how that started happening)... but it's a shame we don't incorporate a failing thumbnail on the frontend into how we cache/check dead links during search. One option for that is in relation to #3585, we could query API request logs in a daily DAG for thumbnails (and waveforms!) that failed over the 24 hour period for the DAG execution date and bust the cached dead link check in redis or go ahead and do a "deep" link verification check using a GET rather than HEAD request. Log querying would be relatively easy and require no new API code, but I can't remember if we already have something in Redis that indicates a thumbnail failed within a certain time period that could be queried instead of logs. The keys from #4249 have a pretty short TTL IIRC, but might be suitable depending. Again, I'm on holiday, so haven't taken the time to look at the code, I opened #4665 during personal usage of Openverse and realised we'd discussed some ways of incorporating the frontend failure. The problem of real-user vs API-level failures still stands when it comes to API logs vs Plausible events. However, in the interest of finding the most immediately usable solution, API-level failures are easy to implement and could theoretically improve search in a meaningful way, without being blocked on the as-of-yet unsolved question of programmatic interactions with Plausible. |
@sarayourfriend I think the log-querying approach is a great idea, and could lead to some really quick wins! On the frontend, I will update this issue to "Reevaluate thumbnail fallback behavior". The "image not avaliable" placeholder is pretty atrocious, with raster text, and could easily be a more-accessible html or svg fallback that matches the styling of the site better. OR, probably better, is to just exclude those results from display. |
Sounds good 👍 I mentioned other options to the "image not available" placeholder in #4665, including removing the result, but it would require a bit of thought to avoid potentially large layout shifts (at least with the current flow of the UI, showing the "skeleton" placeholders before the results load). Replacing them with HTML could be good, might be worth putting that suggestion on #4665 in case it's easy to implement. |
Currently, the if the search results page isn't able to get a thumbnail from the API
/thumbs
endpoint for an image, it falls back to using the full resolution image.In situations where an entire source is unavailable due to an outage, or other problems where thumbnails are unavailable and the full-size images are quite large, this can mean a lot of large images downloading for the user at once and is a very poor user experience.
What are the situations where we would want an image with a failing thumbnail to be included in search results? Typically, a failing thumbnail suggests an issue with the source. It can also simple mean that source images are too large and our thumbnail service isn't able to process them without timing out.
Not sure what the best solution is here, some alternatives I see are:
The text was updated successfully, but these errors were encountered: