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

Fallback for YouTube thumbnails #555

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

nicogaldamez
Copy link
Contributor

Why

What

Adds a method to check if the thumbnail is the generic YouTube thumbnail. It determines this by checking the image size.
image

@adrienpoly
Copy link
Owner

Thank you for investigating this!

While I haven't tested the solution live, my understanding is that for each talk card, we are querying the YouTube domain to verify the thumbnail image. Unfortunately, this approach might not be viable in production since it could severely impact performance.

Given that only a small subset of talks have incorrect thumbnails, it doesn’t make sense to penalize all talk cards with an additional query.

When I briefly explored the issue, I considered a frontend solution. However, since YouTube URLs return a valid image regardless of correctness, there's no error response we can leverage to display a fallback image easily.

Here’s my current suggestion:
We could create a background task to perform a similar check as your current method but store the result in the model. This way, each talk’s thumbnail is verified only once. For talks without thumbnails, we could periodically rerun the task to see if YouTube has updated their image.

@nicogaldamez
Copy link
Contributor Author

Thanks for your response, @adrienpoly !

Have you thought about using cache to handle this?

@adrienpoly
Copy link
Owner

Thanks for your response, @adrienpoly !

Have you thought about using cache to handle this?

I started to look into caching the talk/card yes but this card is not that simple and it does require a bit a rework to improve it's cachability

if your suggestion is to only cache the thumbnail check I think that would create a n+1 and also have a performance impact for the talk index views

@nicogaldamez
Copy link
Contributor Author

I was thinking about caching the whole card, but it’s a pity it’s so complicated.

Since only a small subset of talks have incorrect thumbnails, do you still think it’s worth creating a background task to handle this?

@marcoroth
Copy link
Collaborator

marcoroth commented Jan 7, 2025

I was wondering if we can combine this we the API call we would be doing with #543 to check if the video has the appropriate thumbnails. Or is there no way to figure that out through the API?

@adrienpoly
Copy link
Owner

There are also the view_count and like that we could fetch from the API. This could justify to crawl regularly the YT api and add the thumbnail check if possible

Part of it was added here #22 but we never really went further than just adding the API client

@nicogaldamez
Copy link
Contributor Author

I was wondering if we can combine this we the API call we would be doing with #543 to check if the video has the appropriate thumbnails. Or is there no way to figure that out through the API?

Yes, we can use a HEAD request as it is in the PR

There are also the view_count and like that we could fetch from the API. This could justify to crawl regularly the YT api and add the thumbnail check if possible

I like this! It will enable adding a filter or sort order to view the more popular talks

@adrienpoly adrienpoly force-pushed the main branch 2 times, most recently from 97e3bb3 to 953a362 Compare January 27, 2025 08:53
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.

3 participants