Sync PSF sponsors with logo placement API #10766
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
di
approved these changes
Feb 16, 2022
There was a problem hiding this comment.
LGTM! Just some small comments
I ask reviewers to leave instructions on the new unit tests. I feel that the tests are complex due to pretend and also I feel that probably there's a clever way to mock the task request object.
Tests look good to me, I think your request fixture is fine.
ewdurbin
reviewed
Feb 16, 2022
warehouse/sponsors/__init__.py
Outdated
| @@ -22,3 +24,8 @@ def _sponsors(request): | |||
| def includeme(config): | |||
| # Add a request method which will allow to list sponsors | |||
| config.add_request_method(_sponsors, name="sponsors", reify=True) | |||
|
|
|||
| # Add a periodic task to update sponsors table | |||
| config.add_periodic_task( | |||
There was a problem hiding this comment.
We might want to wrap this in a conditional to check if pythondotorg.api_token is set, then "unconfigure" it in dev. Mostly since I'm going to suggest we run this hourly to reduce the delay for new sponsors and confusion from PSF staff.
ewdurbin
reviewed
Feb 16, 2022
ewdurbin
reviewed
Feb 16, 2022
berinhard
force-pushed
the
feature/sponsors-api-integration
branch
from
35c1b7e
to
d9b7ed3
Compare
ewdurbin
approved these changes
Feb 21, 2022
domdfcoding
pushed a commit
to domdfcoding/warehouse
that referenced
this pull request
Jun 7, 2022
* Add new config variables to integrate with pythondotorg * Add new fields with data from pythondotorg logo placement API * Register task to update sponsors table * Code linter * Fix typo * Minimal working code to integrate with logo palcement endpoint * First unit test to figure out how to mock requests * Create a new sponsor * Add logic to update existing sponsors * Reformat code * Leave HTTP vs HTTPS configuration to be handle by env variable * Fix linter errors * Create sponsor directly from the exception handler * Run task every 10 minutes * There's no need for the if condition * Only schedule cron job if there's a token in the env * Remove pythondotorg env variables from dev * Run linter Co-authored-by: Ee Durbin <ewdurbin@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new scheduled task to keep Pypi's database in sync with Python.org logo placements API. It's scheduled to run every sunday at 03:00 AM. The goal of this work is to centralize sponsors management at pythondotorg and, thus, reduce any type of duplicated work on Pypi side. It also opens the possibility to replace our logo placement by integration with EthicalAds, the same way we did on pythondotorg.
This PR does not handle sidebar, footer, one time or infra structure sponsors. It only operates on top of PSF ones.
I ask reviewers to leave instructions on the new unit tests. I feel that the tests are complex due to
pretendand also I feel that probably there's a clever way to mock the task request object.cc/ @ewdurbin