-
Notifications
You must be signed in to change notification settings - Fork 54
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I tested this locally and opened a few image urls/ thumbnail urls, definitely a significant difference in loading speed. Just one note about legacy non IIF-enabled images.
The thumbnails look great to me, but out of curiosity was there a particular reason for going with 600px
? Historically I think we used 400
.
It's the default width the API is using for thumbnails: I changed the approach to just storing the thumbnail link provided by SMK, these are probably cached so it shouldn't take much time to load. Thumbnails of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, looks great! We'll probably want to keep an eye on it to make sure the new 1024
thumbnail size load times are okay since it's a bit bigger than what we've used in the past, but it looks fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes
Fixes WordPress/openverse#1359 by @AetherUnbound
Fixes WordPress/openverse#1450 by @stacimc
Description
This PR reverts the hard-coding
None
assignment tothumbnail_url
for images to allow provider scripts to set this column. Also, it completes the SMK script with thumbnails as this is the provider that originated the use case, and for ease of testing.This PR enables removing the hotfix to the API applied in WordPress/openverse-api#938 once these images are refreshed.
Testing Instructions
You can try triggering the
smk_workflow
and when it finishes see the image table has thethumbnail
column filled. Recommended to use anAIRFLOW_VAR_INGESTION_LIMIT
(e.g. to 100) for quick testing.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin