[ci] sorting requirements and constraints for raydepsets #58414
Conversation
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
![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 introduces sorting for flags, requirements, and constraints when generating dependency lock files with raydepsets. This is a valuable change for ensuring deterministic and reproducible builds in the CI environment. The core logic change in ci/raydepsets/cli.py is correct, and the updated lock files reflect the intended deterministic ordering. I have one minor suggestion to improve the code's conciseness.
| for requirement in sorted(requirements): | ||
| args.extend([requirement]) |
There was a problem hiding this comment.
This loop can be made more concise and Pythonic. Instead of iterating through the sorted requirements and appending them one by one, you can use list.extend() to add all of them in a single operation.
args.extend(sorted(requirements))![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| args.extend(["-c", constraint]) | ||
| if requirements: | ||
| for requirement in requirements: | ||
| for requirement in sorted(requirements): |
There was a problem hiding this comment.
Bug: Dependency resolution breaks with sorted requirements files
Sorting requirements files alphabetically can change dependency resolution behavior. The order of requirements files matters in pip/uv because later files can override or add constraints from earlier files. By sorting the requirements list, files that were intentionally ordered (e.g., base requirements before specific overrides) may be processed in the wrong order, potentially leading to different dependency versions being resolved. For example, docker/base-deps/requirements.in will now be processed before release/ray_release/byod/ray_dev_py3.10.in due to alphabetical sorting, which may not be the intended precedence order.
ci/raydepsets/cli.py
Outdated
| args.extend(["--cache-dir", self._uv_cache_dir]) | ||
| if override_flags: | ||
| args = _override_uv_flags(override_flags, args) | ||
| args = _override_uv_flags(sorted(override_flags), args) |
There was a problem hiding this comment.
Removing the flag sorting in preparation of building the flag class to handle defaulting, removal and setting flags
There was a problem hiding this comment.
Added unit tests for requirements and constraints
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn
changed the title
elliot-barn
changed the title
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…#58414) Sorting requirements and constraints for raydepsets --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…#58414) Sorting requirements and constraints for raydepsets --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…#58414) Sorting requirements and constraints for raydepsets --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
…#58414) Sorting requirements and constraints for raydepsets --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: YK <1811651+ykdojo@users.noreply.github.com>
…#58414) Sorting requirements and constraints for raydepsets --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sorting requirements and constraints for raydepsets