search

kubeflow / pytorch-operator

PyTorch on Kubernetes
Apache License 2.0
306 stars 143 forks source link

ConvertPyTorchJobToUnstructured uses function ToUnstructured to convert PyTorchJob to Unstructured instead of json #241

Closed leileiwan closed 4 years ago

leileiwan commented 4 years ago

In the PR https://github.com/kubernetes/kubernetes/pull/86291,If we use json to convert PyTorchJob to Unstructured ,the test case will not pass. The code has potential problem.

so, I suggest to use function ToUnstructured to instead of json

k8s-ci-robot commented 4 years ago

Hi @leileiwan. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 22.97% when pulling 771f3d5abb1e17765679e2c762b5ad93188d9a66 on leileiwan:master into 26f9d0ce381fe50926741b776e8a3f73bd77640d on kubeflow:master.

gaocegege commented 4 years ago

/ok-to-test

Thanks for your contribution! :tada: :+1:

/lgtm

johnugeorge commented 4 years ago

@leileiwan Can you also add it to tf-operator repo too? https://github.com/kubeflow/tf-operator/blob/master/pkg/common/util/v1/testutil/util.go

leileiwan commented 4 years ago

@leileiwan Can you also add it to tf-operator repo too? https://github.com/kubeflow/tf-operator/blob/master/pkg/common/util/v1/testutil/util.go

ok,iwill

johnugeorge commented 4 years ago

/approve

k8s-ci-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

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/pytorch-operator/blob/master/OWNERS)~~ [johnugeorge] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment