-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate utilities/provider_tallies
to PDM
#4624
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I have some suggestions but they seem small enough to not warrant blocking the complete move away from Pipenv to PDM.
@@ -51,7 +51,7 @@ def _format_name(value: str) -> str: | |||
type=str, | |||
) | |||
def main(output: Path, start_date: str | None): | |||
redis = Redis("localhost", decode_responses=True, db=TALLY_DATABASE) | |||
redis = Redis("localhost", port=6399, decode_responses=True, db=TALLY_DATABASE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments about this from #4623 apply here too 😕.
Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com>
f8ec396
to
42430c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! No blocking changes but one suggestion for ov
usage.
Co-authored-by: Madison Swain-Bowden <bowdenm@spu.edu>
Awesome, thanks! |
Fixes
Related to #4166 by @dhruvkb
Description
This PR migrates
utilities/provider_tallies
to PDM.Testing Instructions
Run these instructions:
cd utilities/provider_tallies
../../ov pdm install
../../ov pdm run provider_tally_stats.py
You should see an error that you cannot connect to Redis printed in the terminal
If you start Redis locally or connect to the staging Redis cluster, and pass the correct port to the script:
../../ov pdm run provider_tally_stats.py --port=<DEV_REDIS_PORT>
you should see the output of the script in your terminal.
There are some warnings in the output, but that is not related to these changes.
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin