-
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/dead_link
to PDM
#4623
Conversation
utilities/dead_link
to PDM
It's not really necessary for testing, to be clear, and best to avoid if not absolutely necessary to directly connect. You can get the same data locally by requesting search with At the very least, connecting to |
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.
Left one comment but it isn't a blocker. If you'd rather merge this as is, that's fine by me, but a fast-follow issue + PR to make the port and host configurable to discourage using production unless that's very intentional is, I think, a must for this code.
1cb6217
to
01a14d8
Compare
Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com>
e99f8db
to
350507c
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.
This looks more like a full refactor 😄 It worked for me locally with:
../../ov pdm run dead_link_tally.py --port=50263
Great!
Fixes
Related to #4166 by @dhruvkb
Description
This PR migrates the
dead_links
utility to PDM.To make the redis connection parameters (host and port) configurable, it converts the utility to a
click
script with--port
and--host
parameters.It also moves away from a py_redis library that was last updated 3 years ago to
redis
, sets the port for redis connection and also usestqdm.write
to make sure that the progress bar is always at the bottom of the terminal.Testing Instructions
cd utilities/dead_links
../../ov pdm install
../../ov pdm run dead_link_tally.py
You should see the error message saying that you couln't connect to Redis.
Maintainers can also connect to Redis dev server and use the dev port to run
../../ov pdm run dead_link_tally.py --port=DEV_REDIS_PORT
to test running this script with staging data.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