Add domain override functionality for background colors and cover images #594
Conversation
This commit introduces the ability to override the background color and cover image URL for specific domains. This is achieved through the addition of a new `domain_overrides` table in the database and corresponding management scripts. The `perform_overrides` function has been added to apply these overrides when processing cover images. The necessary database configuration has been added to the `config.py` file. The `psycopg2-binary` package has been added to the requirements to handle PostgreSQL database interactions. Unit tests have been updated to cover the new functionality.
There was a problem hiding this comment.
Okay, so assuming we're going to introduce a dashboard for managing this, then this LGTM (otherwise it seems like overkill). It'd be awesome if we could eventually manage all the source information from there, rather than having our sources.*.csv files being the source of truth.
My only other concern is that the image urls tend to change semi regularly, so I'd expect overrides to often 404 after a while. Not sure if there's an easy solution, but maybe we could store the actual image in our DB (though I guess we'd need to serve them ourselves).
bin/manage.py
Outdated
|
|
||
| # if the type is cover_url, the value must be a url | ||
| if type == 'cover_url': | ||
| assert(value[:4] == 'http') |
There was a problem hiding this comment.
Maybe worth enforcing https:?
| @@ -297,6 +299,23 @@ def process_cover_image(item): | |||
|
|
|||
| return domain, padded_image_url, background_color | |||
|
|
|||
| def domain_overrides(config): | |||
| # connect to the database | |||
| db = psycopg2.connect( | |||
There was a problem hiding this comment.
I'm not much of a python programmer, but is there some kind of auto-dispose statement?
# may be something I'm imagining, rather than a real thing but can we do:
with psycopg2.connect(....) as db:
...
return rows|
|
||
| result = perform_overrides(result, overrides) | ||
|
|
||
| assert(result['http://brave.com']['background_color'] == '#000000') |
There was a problem hiding this comment.
Might be worth adding tests that subdomains/other origins don't get overridden.
|
I think we no more needed this PR, Can we close it? @aekeus |
|
Let's wait to see the results of the image work. This may be necessary to change the background colors. |
|
Sorry @aurangzaib048 why don't we need this now? I think fixing the transparency in the cover images will help, but there are definitely images we're going to want to override. For example, this USA Today image is a bandwidth shattering 16x16 And as @aekeus says, we're definitely going to want to override the background color" Once the transparency fix lands, this is going to be a red C on a red background 😆 |
|
@fallaciousreasoning We don’t need this PR, but we need this kind of things for overriding. We are working on the overriding the URL or Background color in different way. Here: https://bravesoftware.slack.com/archives/C05QN258KJ7/p1698155086350469 |
|
@aurangzaib048 I thought the admin dashboard was a GUI to manage the overrides |
|
Yeah that’s correct, but that will be another repo which will be used to do such types of things. @LorenzoMinto |
|
This PR will do the work of injecting the background color or cover url during the aggregation. It is a complement (not a replacement) for the UI @aurangzaib048 . This PR still needs a bit of work (moving the migrations around etc...). |
- Removed domain override management instructions from README.md and added instructions for running tests. - Reordered psycopg2-binary in requirements.dev.txt and requirements.txt for better organization. - Refactored import statements in cover_images.py for better readability. - Improved formatting in cover_images.py and utils.py for better readability. - Updated tests in test.py to include more comprehensive checks for domain overrides.
aekeus
dismissed stale reviews from fallaciousreasoning and LorenzoMinto
via
e010417
aekeus
changed the title
This commit introduces the ability to override the background color and cover image URL for specific domains. This is achieved through the addition of a new
domain_overridestable in a new database and corresponding management scripts. Theperform_overridesfunction has been added to apply these overrides when processing cover images. The necessary database configuration has been added to theconfig.pyfile. Thepsycopg2-binarypackage has been added to the requirements to handle PostgreSQL database interactions. Unit tests have been updated to cover the new functionality.