openverse-api icon indicating copy to clipboard operation
openverse-api copied to clipboard

Deprecate the watermarking APIs and remove fonts from the image

Open dhruvkb opened this issue 3 years ago • 9 comments

The API for watermarking with a frame around the image should be deprecated and removed. It's not used from the frontend and the major migration to Openverse allows us to deprecate a lot of infrequently used code to streamline the repo.

https://github.com/WordPress/openverse-api/blob/59f9092bde555965ff076cb03c22beefcbcb35c2/api/catalog/api/utils/watermark.py#L161

The associated font must be removed from the repository and the Docker image.

https://github.com/WordPress/openverse-api/blob/59f9092bde555965ff076cb03c22beefcbcb35c2/api/Dockerfile#L49

dhruvkb avatar Jan 22 '22 19:01 dhruvkb

@dhruvkb Do we have to remove both the codes given above?

RishiKumarRay avatar Jan 22 '22 19:01 RishiKumarRay

Let's also wait for some more opinions to weigh in on removing the functions based on their low usage.

dhruvkb avatar Jan 23 '22 07:01 dhruvkb

Hmm, this seems like a useful feature and it would be cool to add it to the frontend! But it's also more for us to maintain so I'm fine with deprecating it.

AetherUnbound avatar Jan 24 '22 21:01 AetherUnbound

I would love an endpoint like this for generating open graph images for the search result pages! Something that showed the Openverse logo, name of the image, and the license name and icons. Not dissimilar from this tool I built at creative commons, minus the UI: https://cc-og-image.vercel.app/

zackkrida avatar Jan 25 '22 00:01 zackkrida

@AetherUnbound at CC this feature had a frontend but then it was removed. The code stayed but I'm not too sure if we should keep it as it is. @zackkrida's idea to repurpose it for OG images seems like a good way to salvage it and make it somewhat useful.

dhruvkb avatar Jan 25 '22 03:01 dhruvkb

I'm going to remove the good first issue label for now. We don't have a clear path forward, I don't think this makes sense as someone's first issue to tackle at the moment... maybe once we have an idea of what we actually want to do we can add the label back or create a new issue with clearer definitions of done.

sarayourfriend avatar Feb 03 '22 14:02 sarayourfriend

@WordPress/openverse-maintainers can we re-prioritize this issue to at least 404 the watermark endpoint?

If we want to build a new feature out of the watermark features (which seems good) let's do that in a separate issue.

sarayourfriend avatar Aug 02 '22 12:08 sarayourfriend

While we wait for someone to update this issue based on the discussion during the weekly dev chat from this week, I will also note that we can easily resolve some issues with this endpoint by adding a try/except around the exif.dump line in the endpoint to prevent errors like this one: https://sentry.io/share/issue/a80d52de7f89436586ed0250cd0a32d2/

This is documented here: https://github.com/WordPress/openverse-api/issues/849

sarayourfriend avatar Aug 04 '22 15:08 sarayourfriend

I'm writing the update, given no one else has done so yet.

When thinking about this issue: please keep in mind that if an endpoint exists on a versioned prefix then we have effectively cemented the existence of that endpoint. If we wish to break this contract, then we may do so, but I would ask that we do so with careful consideration and not just to get out of having to fix some issues that appear to be relatively simple bug fixes.

Unless we are prepared to right now commit someone to spending a significant amount of time thinking about and developing an RFC for how we will proceed with API versioning, we should not remove this endpoint. We already have API endpoints that have broken version contracts (deprecated query params without version changes), I do not think we should continue this pattern. In my opinion we do not have the time or expertise to quickly develop an API versioning RFC right now.

In light of that, we should close this issue and solve the handful of bugs in the watermark and oembed endpoints.

sarayourfriend avatar Aug 09 '22 15:08 sarayourfriend

@sarayourfriend your reasoning in your August 9th update is sound. Should we close this issue now? Please do if so.

zackkrida avatar Nov 30 '22 02:11 zackkrida

@zackkrida I'm not sure what the bugs are with this endpoint and was never closely keeping track of them. I'll close this issue but I don't know if the bugs are resolved. @dhruvkb could you confirm that all the bugs for this endpoint are documented in different issues, I seem to recall you being involved in a lot of the discussions about these.

sarayourfriend avatar Nov 30 '22 04:11 sarayourfriend

The issues for the endpoint (#827 and #849) were closed by various PRs (#903 and #932) so afaik, the problems with the endpoint have actually been resolved.

dhruvkb avatar Nov 30 '22 11:11 dhruvkb