kubeflow / training-operator

Distributed ML Training and Fine-Tuning on Kubernetes
https://www.kubeflow.org/docs/components/training
Apache License 2.0
1.62k stars 700 forks source link

KEP-2170: Generate Python SDK for Kubeflow Training V2 #2310

Closed andreyvelich closed 3 weeks ago

andreyvelich commented 3 weeks ago

Part of: https://github.com/kubeflow/training-operator/issues/2216.

I generated kubeflow-training Python SDK for Training V2 APIs. Please let me know what do you think about dir structure and pyproject.toml for the SDK.

/assign @kubeflow/wg-training-leads @varshaprasad96 @akshaychitneni @shravan-achar @deepanker13 @helenxie-bit @Electronic-Waste @saileshd1402 @droctothorpe

coveralls commented 3 weeks ago

Pull Request Test Coverage Report for Build 11536002227

Details


Totals Coverage Status
Change from base Build 11507477280: 0.0%
Covered Lines: 77
Relevant Lines: 77

💛 - Coveralls
tenzen-y commented 3 weeks ago

Could you please explain why we use /sdk/python/kubeflow for SDK v1 but /sdk_v2/kubeflow for SDK v2?

I guess, it might be better if we make the dir structure consistent with v1. WDYT👀 @andreyvelich

Let's revisit the directory structure later. I would not like to block Client implementation. @andreyvelich, Could you open an issue or align the v2 directory structure with the v1 one?

google-oss-prow[bot] commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenzen-y

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubeflow/training-operator/blob/master/OWNERS)~~ [tenzen-y] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
andreyvelich commented 3 weeks ago

@Electronic-Waste @tenzen-y Actually, I did this directory on purpose. Since we don't have plans to introduce SDK in other languages (e.g. Java), we don't need to have python sub-folder.

tenzen-y commented 3 weeks ago

@Electronic-Waste @tenzen-y Actually, I did this directory on purpose. Since we don't have plans to introduce SDK in other languages (e.g. Java), we don't need to have python sub-folder.

If there are not any directory structure limitations for PyPI, I'm ok with the current one.

andreyvelich commented 3 weeks ago

@tenzen-y No, PyPI doesn't have any limitations. As you can see, we just package our kubeflow dir under sdk_v2: https://github.com/kubeflow/training-operator/blob/master/sdk_v2/pyproject.toml#L41

tenzen-y commented 3 weeks ago

@tenzen-y No, PyPI doesn't have any limitations. As you can see, we just package our kubeflow dir under sdk_v2: https://github.com/kubeflow/training-operator/blob/master/sdk_v2/pyproject.toml#L41

Alright. In that case, I'm ok with the current directory structure.