Fix runtime env cache not detecting changes in requirements.txt files referenced via -r#63403
Fix runtime env cache not detecting changes in requirements.txt files referenced via -r#63403Lucas61000 wants to merge 16 commits into
Conversation
… referenced via -r Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
- Restructure file_path logic with explicit None init and if file_path is not None block - Remove delayed imports in tests, move to top-level imports Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
Co-authored-by: Edward Oakes <ed.nmi.oakes@gmail.com> Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
- Restructure file_path logic with explicit None init and if file_path is not None block - Remove delayed imports in tests, move to top-level imports Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
Remove try/except block that was silently catching and logging all exceptions. If a requirements file cannot be read, the exception should propagate up since the hash cannot be calculated and the environment cannot be installed. Also fix ruff linter errors (W293, Q000) in related test file. Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request implements recursive expansion of pip requirements files within the _get_pip_hash function to ensure that changes in nested requirement files correctly trigger hash updates. It includes a new _parse_requirements_file helper and comprehensive unit tests for circular references and relative paths. Feedback indicates that the current implementation is fragile: it lacks proper error handling for missing files (leading to potential FileNotFoundError crashes), does not account for URL-based requirements, and fails to handle empty requirement strings. Additionally, it is recommended to use explicit utf-8 encoding and support inline comments in the requirements parser to improve robustness and cache consistency.
|
Hi @edoakes, I've opened this new PR. Sorry again for the noise caused by the previous one. Could you please take a look when you have a chance? Thanks! |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lucas <54979663+Lucas61000@users.noreply.github.com>
Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 54a911d. Configure here.
|
Premerge CI triggered, if it passes the PR will merge: https://buildkite.com/ray-project/premerge/builds/66623 |
…ders in _get_pip_hash Paths containing unresolved environment variable placeholders (e.g. ) are expanded by the runtime env agent only after the driver-side hash is computed. Such paths are not real files on the driver and attempting to open them causes FileNotFoundError -> HTTP 500 during ray.init(). Skip file expansion for these placeholders and keep the original spec string in the hash input. Real file paths that genuinely don't exist will still propagate FileNotFoundError as required by the earlier review resolution. Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
Head branch was pushed to by a user without write access
|
@edoakes Thanks a lot for the review and approval! I've addressed the issues that could be fixed from the premerge CI feedback. |

Description
When using
-rto reference a requirements.txt file in the runtime environment pip config, Ray doesn't detect changes to the file. This causes the cached environment to be reused even when the file content has changed.Modified
_get_pip_hashinray/_private/runtime_env/pip.pyto:-rreferences in the packages listThis ensures that when the requirements.txt file changes, a new hash is generated and a new runtime environment is created.
Related issues
Closes #61759
Additional information
A re-open new PR from #61760