Remove thumbnails from images

[obulat]

Fixes

Fixes WordPress/openverse#1561 by @obulat

Description

This PR removes thumbnail_url from image provider scripts, and hard-codes thumbnail_url to None in the imageStore class.

Testing Instructions

Run the tests - they should pass. Try running the image DAGs and see that they save image thumbnails as NULL.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[stacimc]

Excited for all this code cleanup! Just a few comments. The only other thumbnail references I'm seeing are in the archive/commoncrawl, which is not being used at the moment and seems like a separate can of worms.

Could we add a test that the thumbnail gets hard-coded to None, maybe in the image store tests?

[obulat]

Could we add a test that the thumbnail gets hard-coded to None, maybe in the image store tests?

I tried adding a specific test for None in the thumbnail, but decided to simply modify existing tests:

• the public method ImageStore.add_image that is used by the provider scripts does not accept the thumbnail_url parameter. If you pass it, it throws an error.
• the inner method ImageStore._get_image is used by the add_image, and by the tests as a short-cut. It was possible to pass a non-null thumbnail_url there (even though only the tests would do it), so I added a line to _get_image that sets thumbnail_url to None anyways. That is tested in one of the tests in test_image.

I wonder if it was better to simply use add_image in tests, without ever using _get_image, which would make the tests more like the usage in the real code. Or maybe it doesn't make sense to call _get_image with arguments that add_image can pass it...

[AetherUnbound]

This is great! I love seeing change counts like this 🤩

This prevents us from acquiring new thumbnail IDs - it would be awesome if we could remove thumbnail_url altogether from the catalog and the API. Will there be a follow up PR to remove that column from the table? I think it would be a good idea to do that shortly after this 😄

[AetherUnbound]

batch_data is going to be a pretty large list in most cases, do you think this could be a logger.debug line?

[obulat]

It was probably left their by mistake :)

[obulat]

This prevents us from acquiring new thumbnail IDs - it would be awesome if we could remove thumbnail_url altogether from the catalog and the API. Will there be a follow-up PR to remove that column from the table? I think it would be a good idea to do that shortly after this 😄

I noted it elsewhere, but will also write it here:
We do use thumbnail_url for other media types, and currently, it is in the list of common columns for all media types. We talked about video and 3D model maybe using thumbnails, too. It might be easier to simply leave it as a common column for all media types, but leave it as NULL for images. What do you think?

[AetherUnbound]

Totally makes sense! 🚀 Do you think it might be worth running an update to set the field to NULL for all existing images?

[obulat]

Totally makes sense! 🚀 Do you think it might be worth running an update to set the field to NULL for all existing images?

Yes, absolutely!

[stacimc]

Looks good! 🎉 Look at that beautiful stat for lines changed 😄