[deps] raydepsets: removing default index url and header #58872
Conversation
Signed-off-by: elliot-barn <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the dependency generation script raydepsets by removing the hardcoded default --index-url and adding a --no-header flag to the uv command. This cleans up the generated lock files by removing the verbose autogeneration comment. While the changes are correct and achieve the stated goal, I have provided a suggestion on ci/raydepsets/cli.py to use a concise custom header instead of removing it entirely. This would improve long-term maintainability by retaining context about how the files are generated. The numerous changes to .lock files are the expected result of this configuration change.
| from ci.raydepsets.workspace import Depset, Workspace | ||
|
|
||
| DEFAULT_UV_FLAGS = """ | ||
| --no-header |
There was a problem hiding this comment.
While --no-header cleans up the lock files, it also removes valuable context about how these files are generated. This can make future maintenance and debugging more difficult. Consider using the --header flag to add a concise, custom header, for example:
# Generated by ci/raydepsets. Do not edit manually.
This provides a pointer to the generation tool without the verbosity of the full command.
| --no-header | |
| --header "# Generated by ci/raydepsets. Do not edit manually." |
Signed-off-by: elliot-barn <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| assert ( | ||
| _override_uv_flags( | ||
| ["--index-url https://dummyurl.com"], | ||
| ["--index-strategy first-index"], |
There was a problem hiding this comment.
Bug: Duplicate test logic after flag removal
The test_override_uv_flag_multiple_flags test is now identical to test_override_uv_flag_single_flag. Both tests override --index-strategy with the same value and perform identical assertions. The test named "multiple_flags" no longer tests multiple flags being overridden simultaneously, making it redundant and not testing what its name suggests.
…#58872) removing flag from raydepsets: --index-url https://pypi.org/simple (included by default https://docs.astral.sh/uv/reference/cli/#uv-pip-compile--default-index) adding flag to raydepsets: --no-header updating unit tests This will prevent updating all lock files updating when default or config level flags are updated --------- Signed-off-by: elliot-barn <[email protected]> Signed-off-by: YK <[email protected]>
…#58872) removing flag from raydepsets: --index-url https://pypi.org/simple (included by default https://docs.astral.sh/uv/reference/cli/#uv-pip-compile--default-index) adding flag to raydepsets: --no-header updating unit tests This will prevent updating all lock files updating when default or config level flags are updated --------- Signed-off-by: elliot-barn <[email protected]>
removing flag from raydepsets: --index-url https://pypi.org/simple (included by default https://docs.astral.sh/uv/reference/cli/#uv-pip-compile--default-index)
adding flag to raydepsets: --no-header
updating unit tests
This will prevent updating all lock files updating when default or config level flags are updated