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.

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

[SDK] Consolidate Naming for CRUD APIs #1877

Closed
andreyvelich opened this issue Aug 2, 2023 · 3 comments · Fixed by #1907
Closed

[SDK] Consolidate Naming for CRUD APIs #1877

andreyvelich opened this issue Aug 2, 2023 · 3 comments · Fixed by #1907

Comments

@andreyvelich
Copy link
Member

Previously, we created unify TrainingClient to give user an ability to create Training Operator Job using single client.
Currently, in our SDK we have mix of different APIs.
For example, to create training job we use job-specific APIs (e.g. create_tfjob), but to get training job logs we use single API despite on job kind (e.g. get_job_logs).
Some of the APIs require job_kind to bet set (e.g. get_job_conditions).

That causes confusions for our users, especially for those who work with single type of Job (e.g. PyTorchJob).
Also, as I mentioned here: #1863 (comment), to get Job events in get_job_logs API, we need to introduce job_kind argument for this API.

Therefore, I propose to make all of these APIs consistent as follows:

  1. We are going to introduce job_kind, argument to our TrainingClient. Similar to our namespace parameter for Katib Client. Thanks to @droctothorpe for this idea!
  2. All CRUD APIs won't be job-specific. For example: create_job, create_job_from_func, delete_job, update_job, etc.
  3. When it is required, we will do appropriate logic depending on job_kind. For example, create the required Training replicas in create_job_from_func API.
  4. job_kind argument can be overridden for every API. So user can use single TrainingClient to create different Training Jobs.

I believe, that should simplify experience of our API usage.

What do you think about this @kubeflow/wg-training-leads @tenzen-y @kuizhiqing @yaobaiwei @zw0610 ?

@terrytangyuan
Copy link
Member

+1 to whatever simplifies user experience and make API consistent

@tenzen-y
Copy link
Member

tenzen-y commented Aug 2, 2023

It's a good proposal. I agree with you, thanks!

@andreyvelich
Copy link
Member Author

/assign @andreyvelich

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants