Skip to content

Conversation

@elliot-barn
Copy link
Contributor

@elliot-barn elliot-barn commented Aug 23, 2025

Including lock file checking as part of raydepsets

@elliot-barn elliot-barn marked this pull request as ready for review August 26, 2025 21:11
@elliot-barn elliot-barn requested a review from aslonnie August 26, 2025 21:11
@elliot-barn elliot-barn changed the title [ci] raydepsets verify lock files [ci] raydepsets check lock files Aug 27, 2025
Signed-off-by: elliot-barn <[email protected]>
@ray-gardener ray-gardener bot added the devprod label Aug 27, 2025
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
@elliot-barn elliot-barn requested a review from aslonnie August 28, 2025 17:22
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
@elliot-barn elliot-barn requested a review from aslonnie September 2, 2025 17:29
+ "".join(diffs)
)
click.echo("Lock files are up to date.")
self.cleanup_temp_dir()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be called by the caller rather than here.

the entity that creates the temp dir should be the one deleting the temp dir.

target_fp,
)

def diff_lock_files(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you check if it is in check mode maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres a check before its called

tags: always
instance_type: small
command: ./ci/test_compile_llm_requirements.sh
command: bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/rayllm.depsets.yaml --check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generated lock files used to be saved in artifacts of the job, so that people can just download it and overwrite locally. are they still being saved as artifacts now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some commands to copy the lock files to the artifact mount

Signed-off-by: elliot-barn <[email protected]>
@elliot-barn elliot-barn requested review from aslonnie and removed request for aslonnie September 4, 2025 23:46
@elliot-barn elliot-barn requested a review from aslonnie September 4, 2025 23:54

def diff_lock_files(self):
diffs = self.get_diffs()
self.cleanup()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, you would want to run cleanup by the caller, the one who constructs the DependencySetManager :)

the one creates it needs to the one who destroys it.

@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Sep 5, 2025
@aslonnie
Copy link
Collaborator

aslonnie commented Sep 5, 2025

not sure why the dashboard test is triggered by this PR..

Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
@elliot-barn elliot-barn requested a review from aslonnie September 6, 2025 00:08
@aslonnie
Copy link
Collaborator

aslonnie commented Sep 6, 2025

test seems failing?

Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the lock file check test on llm deps is failing?

@elliot-barn
Copy link
Contributor Author

  • buildkite/premerge

yes it is let me fix that quickly

Signed-off-by: elliot-barn <[email protected]>
…:ray-project/ray into elliot-barn/raydepsets-verify-lock-files
@elliot-barn
Copy link
Contributor Author

elliot-barn commented Sep 8, 2025

the lock file check test on llm deps is failing?

Fixed. I won't use the --check flag in ci just yet. Will update the ci job in another PR.

Waiting on this PR to get merged before i can use --check directly in the BK job

Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
Signed-off-by: elliot-barn <[email protected]>
@elliot-barn elliot-barn requested a review from aslonnie September 8, 2025 18:21
@aslonnie aslonnie merged commit 1fb6965 into master Sep 8, 2025
5 checks passed
@aslonnie aslonnie deleted the elliot-barn/raydepsets-verify-lock-files branch September 8, 2025 23:47
ZacAttack pushed a commit to ZacAttack/ray that referenced this pull request Sep 24, 2025
Including lock file checking as part of raydepsets

---------

Signed-off-by: elliot-barn <[email protected]>
Co-authored-by: Lonnie Liu <[email protected]>
Signed-off-by: zac <[email protected]>
dstrodtman pushed a commit that referenced this pull request Oct 6, 2025
Including lock file checking as part of raydepsets

---------

Signed-off-by: elliot-barn <[email protected]>
Co-authored-by: Lonnie Liu <[email protected]>
Signed-off-by: Douglas Strodtman <[email protected]>
justinyeh1995 pushed a commit to justinyeh1995/ray that referenced this pull request Oct 20, 2025
Including lock file checking as part of raydepsets

---------

Signed-off-by: elliot-barn <[email protected]>
Co-authored-by: Lonnie Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devprod go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants