Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Deprecate the watermarking APIs and remove fonts from the image #483

Closed
dhruvkb opened this issue Jan 22, 2022 · 12 comments
Closed

Deprecate the watermarking APIs and remove fonts from the image #483

dhruvkb opened this issue Jan 22, 2022 · 12 comments
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix help wanted Open to participation from the community 🟨 priority: medium Not blocking but should be addressed soon 💬 talk: discussion Open for discussions and feedback 🐍 tech: python Requires familiarity with Python 🐛 tooling: sentry Sentry issue

Comments

@dhruvkb
Copy link
Member

dhruvkb commented Jan 22, 2022

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.

def _print_attribution_on_image(img, image_info):

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

ADD catalog/api/utils/fonts/SourceSansPro-Bold.ttf /usr/share/fonts/truetype/SourceSansPro-Bold.ttf

@dhruvkb dhruvkb added good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🛠 goal: fix Bug fix 💻 aspect: code Concerns the software code in the repository 🐍 tech: python Requires familiarity with Python labels Jan 22, 2022
@RishiKumarRay
Copy link

RishiKumarRay commented Jan 22, 2022

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

@dhruvkb
Copy link
Member Author

dhruvkb commented Jan 23, 2022

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

@dhruvkb dhruvkb added the 💬 talk: discussion Open for discussions and feedback label Jan 23, 2022
@AetherUnbound
Copy link
Contributor

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.

@zackkrida
Copy link
Member

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/

@dhruvkb
Copy link
Member Author

dhruvkb commented Jan 25, 2022

@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.

@sarayourfriend
Copy link
Contributor

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
Copy link
Contributor

@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 sarayourfriend added 🟨 priority: medium Not blocking but should be addressed soon 🐛 tooling: sentry Sentry issue and removed 🟩 priority: low Low priority and doesn't need to be rushed labels Aug 2, 2022
@sarayourfriend
Copy link
Contributor

sarayourfriend commented Aug 4, 2022

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: #849

@sarayourfriend
Copy link
Contributor

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.

@zackkrida
Copy link
Member

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

@sarayourfriend
Copy link
Contributor

@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 sarayourfriend closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2022
@dhruvkb
Copy link
Member Author

dhruvkb commented Nov 30, 2022

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix help wanted Open to participation from the community 🟨 priority: medium Not blocking but should be addressed soon 💬 talk: discussion Open for discussions and feedback 🐍 tech: python Requires familiarity with Python 🐛 tooling: sentry Sentry issue
Projects
None yet
Development

No branches or pull requests

5 participants