Added Python API server client #1561

blublinsky · 2023-10-24T10:49:01Z

Why are these changes needed?

The current Python client is k8-based. This new Python client requires less k8 knowledge from the user and thus lowers the entry barrier for Python developers to use the operator.
Additionally, it cleans up some of the current definitions of the API server

Related issue number

Checks

[x ] I've made sure the tests are passing.
Testing Strategy
- [x ] Unit tests
- Manual tests
- This PR is not tested :(

blublinsky · 2023-10-24T10:52:28Z

@z103cb, @tedhtchang, @astefanutti please take a look

cc @kevin85421

clients/__init__.py

clients/python-apiserver-client/.gitignore

blublinsky · 2023-10-25T12:55:47Z

@anishasthana pls take a look if you have time

z103cb

Overall this is looking good, but I am not a python expert.

maxdebayser

I think a lot of data object code could be trimmed using pydantic. Is there a constraint on bringing in new external dependencies?

The client code should ideally wrap the HTTP calls using the requests library in another method so that when a refactoring of http calls to add authentication is needed, the change can be done in a single place.

maxdebayser · 2023-10-30T13:44:31Z

clients/python-apiserver-client/python_apiserver_client/params/headnode.py

+        self.annotations = annotations
+        self.labels = labels
+
+    def to_string(self) -> str:


Wouldn't it be simpler to use something like pydantic for these dataclasses? Then you would get the to_string and to_dict methods for free.

Not really, If you notice I do names transformations between classes and dict. I want to be able to use different names.
Additionally I am separating input and output params in to_dict to ensure the proper input. Finally I do not think pydantic can do complex back transformations from dict

Usage of pydantic will require custom serializers, so its basically the same amount of code if not more

If all the transformations you need are between snake case and camel case it's not that much code: https://medium.com/analytics-vidhya/camel-case-models-with-fast-api-and-pydantic-5a8acb6c0eee

Lots of boilerplate here...

tedhtchang

I was able to verify the PR by running the tests cases. We may need a little more instructions in the README.

clients/python-apiserver-client/README.md

clients/python-apiserver-client/pyproject.toml

clients/python-apiserver-client/README.md

Signed-off-by: Boris Lublinsky <blublinsky@hotmail.com>

…_python # Conflicts: # apiserver/Volumes.md # clients/python-apiserver-client/README.md # clients/python-apiserver-client/python_apiserver_client/kuberay_apis.py # proto/go_client/cluster.pb.go # proto/go_client/config.pb.go

blublinsky · 2023-10-31T16:06:16Z

@kevin85421 I think we are good with this and can merge

maxdebayser

Since the benefit of removing some code lines versus the cost including a new dependency like pydantic is not clear, I think this is fine.

deanwampler

Suggestions for spelling grammar, etc., but I also asked if much of the more "boilerplate" part of this API could be replaced by some standard Python for K8s. It seems that many concepts here are not specific to Ray or KubeRay.

apiserver/pkg/model/volumes_test.go

clients/python-apiserver-client/README.md

clients/python-apiserver-client/python_apiserver_client/kuberay_apis.py

clients/python-apiserver-client/python_apiserver_client/params/environmentvariables.py

clients/python-apiserver-client/python_apiserver_client/params/headnode.py

deanwampler · 2023-10-31T18:42:01Z

clients/python-apiserver-client/python_apiserver_client/params/headnode.py

+        self.annotations = annotations
+        self.labels = labels
+
+    def to_string(self) -> str:


Lots of boilerplate here...

clients/python-apiserver-client/python_apiserver_client/params/templates.py

clients/python-apiserver-client/python_apiserver_client/params/volumes.py

blublinsky · 2023-10-31T20:43:02Z

of this API could be replaced by some standard Python for K8s. It seems that many concepts here are not specific to Ray or KubeRay.

Do not confuse Python APIs with Go implementation, which is using standard Go k8 APIs

tedhtchang

All tests seem to pass. One spelling fix left over. I think in a separate PR we may add the unit test to the apiserver e2e test.

blublinsky · 2023-11-01T18:27:41Z

@kevin85421 There are 3 approvals already, lets merge it

kevin85421

Stamp

Added Python API server client

8dc50ab

z103cb reviewed Oct 24, 2023

View reviewed changes

clients/__init__.py Outdated Show resolved Hide resolved

updated _init_.py

4532036

z103cb suggested changes Oct 25, 2023

View reviewed changes

clients/__init__.py Outdated Show resolved Hide resolved

blublinsky added 4 commits October 25, 2023 09:07

Add setup tools for build

d7e4505

Add setup tools for build

4a511d9

Add setup tools for build

858cb3e

Add setup tools for build

aeef883

z103cb reviewed Oct 25, 2023

View reviewed changes

clients/python-apiserver-client/.gitignore Show resolved Hide resolved

package restructuring and local module build

ac9ff0d

blublinsky added 4 commits October 25, 2023 15:54

more cleanup

d71c134

more testing

99344ef

more testing

11e022d

more testing

aab4009

z103cb reviewed Oct 27, 2023

View reviewed changes

blublinsky mentioned this pull request Oct 27, 2023

Add end to end tests to apiserver #1460

Merged

5 tasks

maxdebayser reviewed Oct 30, 2023

View reviewed changes

tedhtchang reviewed Oct 30, 2023

View reviewed changes

blublinsky added 10 commits October 31, 2023 03:31

Merge branch 'master' into apiserver_python

503eb85

Signed-off-by: Boris Lublinsky <blublinsky@hotmail.com>

Added Python API server client

75b6674

updated _init_.py

e9aab6b

Add setup tools for build

47be601

Add setup tools for build

ad957b0

Add setup tools for build

91b0428

Add setup tools for build

b20fe22

package restructuring and local module build

ddcd920

more cleanup

5bdbbaa

more testing

74882db

blublinsky added 5 commits October 31, 2023 14:00

more testing

f2be3f6

more testing

f1ba2d1

rebased and fixed APIs to use v1

e388e58

rebased and fixed APIs to use v1

8b1456c

maxdebayser approved these changes Oct 31, 2023

View reviewed changes

deanwampler reviewed Oct 31, 2023

View reviewed changes

rebased and fixed APIs to use v1

0df8186

tedhtchang approved these changes Nov 1, 2023

View reviewed changes

rebased and fixed APIs to use v1

a1c097f

kevin85421 self-assigned this Nov 1, 2023

kevin85421 added the 1.0 label Nov 1, 2023

kevin85421 approved these changes Nov 1, 2023

View reviewed changes

kevin85421 merged commit 1f728c5 into ray-project:master Nov 1, 2023
22 of 23 checks passed

blublinsky deleted the apiserver_python branch November 2, 2023 12:17

kevin85421 pushed a commit to kevin85421/kuberay that referenced this pull request Nov 2, 2023

Added Python API server client (ray-project#1561)

c89d21c

kevin85421 added a commit that referenced this pull request Nov 2, 2023

Added Python API server client (#1561) (#1616)

0725be3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Python API server client #1561

Added Python API server client #1561

blublinsky commented Oct 24, 2023

blublinsky commented Oct 24, 2023

blublinsky commented Oct 25, 2023

z103cb left a comment

maxdebayser left a comment

maxdebayser Oct 30, 2023

blublinsky Oct 30, 2023

blublinsky Oct 30, 2023

maxdebayser Oct 30, 2023

deanwampler Oct 31, 2023

tedhtchang left a comment

blublinsky commented Oct 31, 2023

maxdebayser left a comment

deanwampler left a comment

deanwampler Oct 31, 2023

blublinsky commented Oct 31, 2023

tedhtchang left a comment

blublinsky commented Nov 1, 2023

kevin85421 left a comment

Added Python API server client #1561

Added Python API server client #1561

Conversation

blublinsky commented Oct 24, 2023

Why are these changes needed?

Related issue number

Checks

blublinsky commented Oct 24, 2023

blublinsky commented Oct 25, 2023

z103cb left a comment

Choose a reason for hiding this comment

maxdebayser left a comment

Choose a reason for hiding this comment

maxdebayser Oct 30, 2023

Choose a reason for hiding this comment

blublinsky Oct 30, 2023

Choose a reason for hiding this comment

blublinsky Oct 30, 2023

Choose a reason for hiding this comment

maxdebayser Oct 30, 2023

Choose a reason for hiding this comment

deanwampler Oct 31, 2023

Choose a reason for hiding this comment

tedhtchang left a comment

Choose a reason for hiding this comment

blublinsky commented Oct 31, 2023

maxdebayser left a comment

Choose a reason for hiding this comment

deanwampler left a comment

Choose a reason for hiding this comment

deanwampler Oct 31, 2023

Choose a reason for hiding this comment

blublinsky commented Oct 31, 2023

tedhtchang left a comment

Choose a reason for hiding this comment

blublinsky commented Nov 1, 2023

kevin85421 left a comment

Choose a reason for hiding this comment