[Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests #1050

Yicheng-Lu-llll · 2023-04-25T23:29:35Z

Why are these changes needed?

From v0.5.0, we inject --block in the rayStartParams automatically in #932. We remove block from YAML files after release v0.5.0 to follow the compatibility philosophy (#940) of KubeRay.

This PR:

Removes the block from rayStartParams for Python client and KubeRay operator tests
Adds corresponding ci test.

Related issue number

Closes #971
#940

Checks

I have run all tests and sample files for python client.

# Under /home/ubuntu/kuberay/clients/python-client
pip3 install -e .
python3 -m unittest discover './python_client_test/'

for file in ./examples/*.py
do
    echo "Running $file"
    python3 "$file"
done

I have run KubeRay operator tests:

# under /home/ubuntu/kuberay/ray-operator
make test

kevin85421

Add an unit test for --block option:

case 1: rayStartParams contains --block
case 2: rayStartParams does not contain --block.

kevin85421 · 2023-04-26T21:18:56Z

cc @jasoonn would you mind reviewing this PR? Thanks!

jasoonn · 2023-04-27T05:01:29Z

LGTM. Thanks for the contribution!

ray-operator/controllers/ray/common/pod_test.go

clients/python-client/python_client/utils/kuberay_cluster_utils.py

clients/python-client/examples/use-raw-with-api.py

clients/python-client/python_client_test/test_api.py

clients/python-client/python_client_test/test_utils.py

kevin85421 · 2023-04-29T03:21:07Z

clients/python-client/python_client_test/test_utils.py

+            group_name="small-group",
+            ray_image="rayproject/ray:2.3.0",
+            ray_command=["/bin/bash", "-lc"],
+            init_image="busybox:1.28",


Why do we need this?

Thank you for highlighting this! Since init_image is now essentially unused in the Python client, I have also removed this argument for the Python client accordingly.

clients/python-client/python_client_test/test_utils.py

ray-operator/controllers/ray/common/pod_test.go

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

kevin85421 · 2023-05-02T00:51:56Z

clients/python-client/python_client/utils/kuberay_cluster_builder.py

@@ -121,17 +120,14 @@ def build_worker(
        group_name: str,
        ray_image: str = "rayproject/ray:2.4.0",
        ray_command: Any = ["/bin/bash", "-lc"],
-        init_image: str = "busybox:1.28",


@jasoonn @akanso is this a user-facing function? Thanks!

Yes. It gives users the ability to customize their workers' configuration.

The build_worker() passes init_image as a parameter to call populate_worker_group() to generate worker group configuration. However, populate_worker_group() does not actually use that parameter. Hence, it makes sense to me to remove init_image here.

If build_worker is a user-facing function, this change seems to break the backward compatibility. Is it OK?

It's not okay. I forgot to take backward compatibility into consideration.

It breaks the backward compatibility but disruption is expected to be minimal.

init_image has a default value and in most cases, there is no strong need to modify it considering its meaning. Also for this reason, init_image does not appear in any Python client examples.

Let's keep init_image unchanged on this PR and make a separate PR to discuss this issue.

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

kevin85421

LGTM. @jasoonn we decide to keep init_image in the Python client module the same. Would you mind opening a PR to discuss whether we can break the backward compatibility? Thanks!

kevin85421 · 2023-05-08T20:08:50Z

This PR has already been merged (ba814ef), but I am not sure why this PR has not been closed. Close this PR manually.

…ent and KubeRay operator tests (ray-project#1050) Remove block from rayStartParams for python client and KubeRay operator tests

jasoonn · 2023-05-09T06:13:05Z

LGTM. @jasoonn we decide to keep init_image in the Python client module the same. Would you mind opening a PR to discuss whether we can break the backward compatibility? Thanks!

No problem. I have opened the issue #1072.

…ent and KubeRay operator tests (ray-project#1050) Remove block from rayStartParams for python client and KubeRay operator tests

Yicheng-Lu-llll changed the title ~~[Post release v0.5.0] Remove block from rayStartParams for python client~~ [Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests Apr 26, 2023

kevin85421 reviewed Apr 26, 2023

View reviewed changes

kevin85421 reviewed Apr 28, 2023

View reviewed changes

kevin85421 reviewed Apr 29, 2023

View reviewed changes

clients/python-client/python_client_test/test_utils.py Outdated Show resolved Hide resolved

ray-operator/controllers/ray/common/pod_test.go Outdated Show resolved Hide resolved

Yicheng-Lu-llll force-pushed the remove-block-true branch from 6aa2018 to 92450cb Compare April 29, 2023 18:50

Remove block

dbfd86c

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

Yicheng-Lu-llll force-pushed the remove-block-true branch from 92450cb to dbfd86c Compare April 29, 2023 18:57

Yicheng-Lu-llll added 2 commits April 29, 2023 19:02

remove block for KubeRay operator tests

a5dc0e5

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

nit

b33c721

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

kevin85421 reviewed May 2, 2023

View reviewed changes

Yicheng-Lu-llll added 2 commits May 5, 2023 17:53

revert back init_image

9b60321

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

revert back init_image

0e39ac4

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>

kevin85421 self-assigned this May 8, 2023

kevin85421 approved these changes May 8, 2023

View reviewed changes

kevin85421 closed this May 8, 2023

kevin85421 mentioned this pull request May 8, 2023

[Feature] Remove block: true in configuration YAML files after release 0.5.0 #971

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests #1050

[Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests #1050

Yicheng-Lu-llll commented Apr 25, 2023 •

edited

Loading

kevin85421 left a comment

kevin85421 commented Apr 26, 2023

jasoonn commented Apr 27, 2023

kevin85421 Apr 29, 2023

Yicheng-Lu-llll Apr 29, 2023

kevin85421 May 2, 2023

jasoonn May 2, 2023

jasoonn May 2, 2023

kevin85421 May 2, 2023

jasoonn May 3, 2023

Yicheng-Lu-llll May 3, 2023

Yicheng-Lu-llll May 5, 2023

kevin85421 left a comment

kevin85421 commented May 8, 2023

jasoonn commented May 9, 2023

[Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests #1050

[Post release v0.5.0] Remove block from rayStartParams for python client and KubeRay operator tests #1050

Conversation

Yicheng-Lu-llll commented Apr 25, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

kevin85421 left a comment

Choose a reason for hiding this comment

kevin85421 commented Apr 26, 2023

jasoonn commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin85421 left a comment

Choose a reason for hiding this comment

kevin85421 commented May 8, 2023

jasoonn commented May 9, 2023

Yicheng-Lu-llll commented Apr 25, 2023 •

edited

Loading