search

kubeflow / common

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

Move Pods/Services control interface to separate folder #72

Closed Jeffwan closed 4 years ago

Jeffwan commented 4 years ago

Currently, pods/services implementation mixed with jobController together under common folder, this is a little messy.

The reason we want to have separate folder (package name) for it are

  1. PodControlInterface is folked from controller_util.go and we also implement similar codes for service. They are logically separate. For long term, once https://github.com/kubernetes/client-go/issues/332 is merged, we could remove the folder entirely.

  2. Since PodControlInterface and ServiceControlInterface are internal code base for common project, there's no need for users to import them at all.

  3. Trying to make some changes inside control to solve #48. It would be better to file this PR first and make changes against new folder, that would be easier for review. Otherwise, it's a new file.

kubeflow-bot commented 4 years ago

This change is Reviewable

Jeffwan commented 4 years ago

/assign @terrytangyuan

k8s-ci-robot commented 4 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/master/OWNERS)~~ [terrytangyuan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
terrytangyuan commented 4 years ago

@Jeffwan We also need to update the import statement in xgboost-operator

Jeffwan commented 4 years ago

@terrytangyuan What's the statement? Do you need me to help update common there?

terrytangyuan commented 4 years ago

@Jeffwan Never mind. It looks like there is no public interface change yet so I am only updating the version of common in https://github.com/kubeflow/xgboost-operator/pull/47.

Jeffwan commented 4 years ago

@Jeffwan Never mind. It looks like there is no public interface change yet so I am only updating the version of common in kubeflow/xgboost-operator#47.

Leave you comment on the PR.