Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Move Pods/Services control interface to separate folder #72

Merged
merged 2 commits into from
Apr 15, 2020

Conversation

Jeffwan
Copy link
Member

@Jeffwan Jeffwan commented Apr 15, 2020

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 Adding expectations and controller ref manager to client-go kubernetes/client-go#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 Delete direct dependency of kubernetes #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.

Even we replace all kubernetes dependencies in go.mod, we want to use lowest version to avoid potential dependency issues. See kubeflow#70 for more details.

Signed-off-by: Jiaxin Shan <[email protected]>
@kubeflow-bot
Copy link

This change is Reviewable

@Jeffwan
Copy link
Member Author

Jeffwan commented Apr 15, 2020

/assign @terrytangyuan

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@k8s-ci-robot
Copy link

[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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit b8d5dca into kubeflow:master Apr 15, 2020
@terrytangyuan
Copy link
Member

terrytangyuan commented Apr 15, 2020

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

@Jeffwan Jeffwan deleted the control branch April 15, 2020 21:32
@Jeffwan
Copy link
Member Author

Jeffwan commented Apr 15, 2020

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

@terrytangyuan
Copy link
Member

@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.

@Jeffwan
Copy link
Member Author

Jeffwan commented Apr 16, 2020

@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.

georgkaleido pushed a commit to georgkaleido/common that referenced this pull request Jun 9, 2022
* Update config.yml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants