Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Code restructuring to support V1alpha1 and V1alpha2 API #448

Merged
merged 6 commits into from
Apr 1, 2019
Merged

Code restructuring to support V1alpha1 and V1alpha2 API #448

merged 6 commits into from
Apr 1, 2019

Conversation

johnugeorge
Copy link
Member

@johnugeorge johnugeorge commented Mar 30, 2019 •
edited
Loading

Code is restructured to support v1alpha1 and v1alpha2 api. Docker images of each component contains v1alpha1 and v1alpha2 binaries(Currently added only for controller and db components). Default is still v1alpha1 binary.

  1. v1alpha2 Controller and DB Bolierplate code is added. I will add the initial implementation in a separate PR
  2. v1alpha2 version of Suggestion is not added yet. Implementation can be merged with Move all suggestion algorithms to python #407 which tracks the python migration of algorithms
  3. GRPC API Boilerplate structure is added. Related: [WIP] Decoupling DB interface from vizier manager #431.
  4. v1alpha2 test structure is not added. It can be added once all v1alpha2 components are created.

Related: #670

This change is Reviewable

@johnugeorge
Copy link
Member Author

/retest

@johnugeorge johnugeorge changed the title WIP: Code restructuring to support V1alpha1 and V1alpha2 API Code restructuring to support V1alpha1 and V1alpha2 API Mar 31, 2019
@johnugeorge
Copy link
Member Author

@richardsliu @YujiOshima @hougangliu @gaocegege @jdplatt @achalshant

@johnugeorge johnugeorge mentioned this pull request Mar 31, 2019
Closed
gaocegege
gaocegege reviewed Mar 31, 2019
View reviewed changes
Copy link
Member

@gaocegege gaocegege left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks for your contribution!

@richardsliu
Copy link
Contributor

/lgtm
Thanks!

@richardsliu
Copy link
Contributor

For the NAS files - we should make sure their naming follow the conventions of the rest of the repo. It can be fixed later.

andreyvelich
andreyvelich reviewed Mar 31, 2019
View reviewed changes
cmd/studyjobcontroller/v1alpha2/main.go Outdated
"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
)

func main() {
Copy link
Member

Choose a reason for hiding this comment

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

Will Katib controller have hame "StudyJobController" in v1alpha2?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@andreyvelich
Copy link
Member

@johnugeorge Thank you for your PR!

@andreyvelich
Copy link
Member

@johnugeorge Should we also put these manifests https://github.com/kubeflow/katib/tree/master/manifests in v1alpha1 folder?

@hougangliu
Copy link
Member

/lgtm
thanks!

@YujiOshima
Copy link
Contributor

/lgtm
Thank you!
As @andreyvelich pointed, we should think about the folder name of cmd/studyjobcontroller later.

@k8s-ci-robot k8s-ci-robot removed the lgtm label Apr 1, 2019
@johnugeorge
Copy link
Member Author

/retest

@johnugeorge
Copy link
Member Author

renamed studyjobcontroller to katib-controller

Image name is not changed yet. It will be changed later when all v1alpha2 components are built.vizier term in image names will also be renamed to katib

I will raise a separate PR for separating v1alpha1 in manifests, scripts, tests folders

@YujiOshima
Copy link
Contributor

/lgtm
Thanks!

@richardsliu
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardsliu

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 3d4cd04 into kubeflow:master Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreyvelich andreyvelich andreyvelich left review comments

@gaocegege gaocegege gaocegege left review comments

@jose5918 jose5918 Awaiting requested review from jose5918

Assignees

@gaocegege gaocegege

@YujiOshima YujiOshima

@hougangliu hougangliu

@richardsliu richardsliu

Labels
approved lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@johnugeorge @richardsliu @andreyvelich @hougangliu @YujiOshima @k8s-ci-robot @gaocegege