Skip to content
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.

Sign up for GitHub

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

[Core] Add ~/.local/bin to make which ray work if ray is installed in ~/.local #3368

Merged
merged 12 commits into from
Apr 15, 2024
Merged

[Core] Add ~/.local/bin to make which ray work if ray is installed in ~/.local #3368

Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d8fecfe
save ray path
Michaelvll Mar 26, 2024
98fffc4
only use path in file when non empty
Michaelvll Mar 26, 2024
74b9d1e
try grepping but fail
Michaelvll Mar 26, 2024
9108510
hardcode ~/.local/bin
Michaelvll Mar 26, 2024
14d9eae
format
Michaelvll Mar 26, 2024
efcd14c
add comments
Michaelvll Mar 29, 2024
b73787b
Merge branch 'master' of github.com:skypilot-org/skypilot into ray_path
Michaelvll Apr 13, 2024
a2a2d1e
Add comments
Michaelvll Apr 14, 2024
48e57e2
format
Michaelvll Apr 14, 2024
4b40d30
avoid backward compat test conflict
Michaelvll Apr 14, 2024
e03e7a9
fix sleep
Michaelvll Apr 14, 2024
19985e9
Fix the task ID for spot pipeline
Michaelvll Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions sky/skylet/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@
# used for installing SkyPilot runtime (ray and skypilot).
SKY_PYTHON_PATH_FILE = '~/.sky/python_path'
SKY_RAY_PATH_FILE = '~/.sky/ray_path'
SKY_GET_PYTHON_PATH_CMD = (f'cat {SKY_PYTHON_PATH_FILE} 2> /dev/null || '
SKY_GET_PYTHON_PATH_CMD = (f'[ -s {SKY_PYTHON_PATH_FILE} ] && '
f'cat {SKY_PYTHON_PATH_FILE} 2> /dev/null || '
'which python3')
# Python executable, e.g., /opt/conda/bin/python3
SKY_PYTHON_CMD = f'$({SKY_GET_PYTHON_PATH_CMD})'
SKY_PIP_CMD = f'{SKY_PYTHON_CMD} -m pip'
# Ray executable, e.g., /opt/conda/bin/ray
SKY_RAY_CMD = f'$(cat {SKY_RAY_PATH_FILE} 2> /dev/null || which ray)'
SKY_RAY_CMD = (f'$([ -s {SKY_RAY_PATH_FILE} ] && '
f'cat {SKY_RAY_PATH_FILE} 2> /dev/null || which ray)')

# The name for the environment variable that stores the unique ID of the
# current task. This will stay the same across multiple recoveries of the
Expand Down Expand Up @@ -121,12 +123,18 @@
# latest ray port 6380, but those existing cluster launched before #1790
# that has ray cluster on the default port 6379 will be upgraded and
# restarted.
'echo PATH=$PATH; '
Copy link
Member

Choose a reason for hiding this comment

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

Is this for debugging? Worth a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, just added the comment.

f'{SKY_PIP_CMD} list | grep "ray " | '
f'grep {SKY_REMOTE_RAY_VERSION} 2>&1 > /dev/null '
f'|| {RAY_STATUS} || '
f'{SKY_PIP_CMD} install --exists-action w -U ray[default]=={SKY_REMOTE_RAY_VERSION};' # pylint: disable=line-too-long
f'{SKY_PIP_CMD} install --exists-action w -U ray[default]=={SKY_REMOTE_RAY_VERSION}; ' # pylint: disable=line-too-long
# Add missing PATH to make sure ray is in the PATH, when the
# previous ray installation happens in user's `~/.local` directory.
# ~/.local/bin is added to the end of PATH to avoid conflicts with ray just
# installed in the conda environment.
'export PATH=$PATH:$HOME/.local/bin; '
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand correctly:

  • The issue is in some envs, our own Ray is by default installed into ~/.local/bin
  • In that env, this path is not in $PATH by default
  • So, previously we touch the SKY_RAY_PATH_FILE file, which is empty
  • Bug is, later on when we do our_ray start, our_ray is basically empty string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is exactly the issue. Tried to elaborate the comments a bit. PTAL : )

# Writes ray path to file if it does not exist or the file is empty.
f'[ -s {SKY_RAY_PATH_FILE} ] || which ray > {SKY_RAY_PATH_FILE};'
f'[ -s {SKY_RAY_PATH_FILE} ] || which ray > {SKY_RAY_PATH_FILE}; '
# END ray package check and installation
f'{{ {SKY_PIP_CMD} list | grep "skypilot " && '
'[ "$(cat ~/.sky/wheels/current_sky_wheel_hash)" == "{sky_wheel_hash}" ]; } || ' # pylint: disable=line-too-long
Expand Down
Loading