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

Remove SourceLogo class #986

Closed
1 task
sarayourfriend opened this issue Oct 24, 2022 · 2 comments · Fixed by #987
Closed
1 task

Remove SourceLogo class #986

sarayourfriend opened this issue Oct 24, 2022 · 2 comments · Fixed by #987
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon

Comments

@sarayourfriend
Copy link
Contributor

Problem

It doesn't appear that we use this data in production... I checked a handful of providers (including big ones like Wikimedia and other smaller ones) and none had an image uploaded. Currently, the uploaded SourceLogo objects get stored directly on disk and get wiped out on each deployment 😱 That leads me further to believe that these are not used or depended on by anyone, as they do not appear to work correctly in production (at least not to my naive expectations of these, that they would persist across deployments) and our current setup doesn't accommodate them at all.

Description

Can we remove this functionality? Has it ever been used in production/what was the intended use case for it (aside from providing a source logo URL along with the individual provider metadata)?

Removing it would simplify the Django API infrastructure in that it would allow us to avoid needing to map any volumes between the Nginx and Django containers once #821 is completed.

Plus, if we can avoid needing to handle uploads (and backing them up!) then that would be great as well.

Alternatives

Additional context

Implementation

  • 🙋 I would be interested in implementing this feature.
@sarayourfriend sarayourfriend added 🟨 priority: medium Not blocking but should be addressed soon 💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users labels Oct 24, 2022
@krysal
Copy link
Member

krysal commented Oct 24, 2022

I think it what intended to show the logos on the homepage in the CC Search times but was never put into use. I don't foresee a use case for this in the near future so seems like can be removed.

Edit: Found a mockup of a "Browse collections" feature of that time. The logos are a good touch. I guess all boils down to when we want to have something like this, but would be nice to have the basic API ECS deployment before.

@AetherUnbound
Copy link
Contributor

Oh, that's a great find Krystle! I could also imagine that we might just store a URL to a logo if we wanted this in the future, rather than storing the URLs ourselves 😮 Either way, I think we'd be clear to remove it 🙂

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: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants