search

kubeflow / common

Common APIs and libraries shared by other Kubeflow operator repositories.
Apache License 2.0
51 stars 73 forks source link

Revert #135: change rtype to commonv1.ReplicaType #158

Closed Jeffwan closed 3 years ago

Jeffwan commented 3 years ago

https://github.com/kubeflow/common/pull/135 This change brings some extra side effects and make training operator dependency upgrade fail. Since this change is kind of refactor, we determine to revert it at this moment.

/cc @gaocegege @terrytangyuan @kubeflow/wg-training-leads

Need two of you to approve the change.

I kick off an end to end test in tf-operator to verify this is working. https://github.com/kubeflow/tf-operator/pull/1395

This PR unblocks https://github.com/kubeflow/tf-operator/pull/1388 and https://github.com/kubeflow/tf-operator/pull/1383

Jeffwan commented 3 years ago

no build issues now. 

➜  common git:(revert_rtype_change) ✗ go build ./...
➜  common git:(revert_rtype_change) ✗ go test ./...
?       github.com/kubeflow/common/pkg/apis/common/v1   [no test files]
ok      github.com/kubeflow/common/pkg/controller.v1/common 0.018s
ok      github.com/kubeflow/common/pkg/controller.v1/control    0.033s
ok      github.com/kubeflow/common/pkg/controller.v1/expectation    0.023s
ok      github.com/kubeflow/common/pkg/util 0.020s
?       github.com/kubeflow/common/pkg/util/k8sutil [no test files]
?       github.com/kubeflow/common/pkg/util/signals [no test files]
ok      github.com/kubeflow/common/pkg/util/train   0.013s
?       github.com/kubeflow/common/test_job/apis/test_job/v1    [no test files]
?       github.com/kubeflow/common/test_job/client/clientset/versioned  [no test files]
?       github.com/kubeflow/common/test_job/client/clientset/versioned/fake [no test files]
?       github.com/kubeflow/common/test_job/client/clientset/versioned/scheme   [no test files]
?       github.com/kubeflow/common/test_job/client/clientset/versioned/typed/test_job/v1    [no test files]
?       github.com/kubeflow/common/test_job/client/clientset/versioned/typed/test_job/v1/fake   [no test files]
?       github.com/kubeflow/common/test_job/client/informers/externalversions   [no test files]
?       github.com/kubeflow/common/test_job/client/informers/externalversions/internalinterfaces    [no test files]
?       github.com/kubeflow/common/test_job/client/informers/externalversions/test_job  [no test files]
?       github.com/kubeflow/common/test_job/client/informers/externalversions/test_job/v1   [no test files]
?       github.com/kubeflow/common/test_job/client/listers/test_job/v1  [no test files]
?       github.com/kubeflow/common/test_job/controller.v1/test_job  [no test files]
?       github.com/kubeflow/common/test_job/test_util/v1    [no test files]
google-oss-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

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/common/blob/release-0.3/OWNERS)~~ [terrytangyuan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
zw0610 commented 3 years ago

@Jeffwan For the master branch, does it means we should still use commonv1.ReplicaType for argument type? I was updating the common repo to master branch in tf-operator to use the reconciler package, but it breaks SetClusterSpec method.