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.

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

api: OpenAPI support #577

Merged
merged 6 commits into from
May 31, 2018
Merged

api: OpenAPI support #577

merged 6 commits into from
May 31, 2018

Conversation

gaocegege
Copy link
Member

@gaocegege gaocegege commented May 9, 2018

Signed-off-by: Ce Gao gaoce@caicloud.io


This change is Reviewable

@gaocegege
Copy link
Member Author

Ref kubernetes/kube-openapi#13

@coveralls
Copy link

coveralls commented May 9, 2018

Coverage Status

Coverage remained the same at 55.677% when pulling 6851cf1 on gaocegege:openapi into a9294ce on kubeflow:master.

@gaocegege
Copy link
Member Author

gaocegege commented May 9, 2018

It seems that I misunderstand the meaning of the beta feature CustomResourceValidation. We do not have to generate/serve open api spec to use the feature.

We could use OpenAPI to generate clients for all languages to allow users to CRUD tfjob in all languages. While the feature CustomResourceValidation is to let the API server to use `` to validate the config.

Here is the logic, and we do not need to have a OpenAPI spec for the feature.

@jlewi Do you think we need to generate clients for all languages to allow users to CRUD tfjob in all languages?

@jlewi
Copy link
Contributor

jlewi commented May 10, 2018 via email

@gaocegege
Copy link
Member Author

SGTM

@gaocegege gaocegege changed the title WIP api: OpenAPI support api: OpenAPI support May 28, 2018
@gaocegege
Copy link
Member Author

@jlewi

I am trying to generate crd validation with the support of https://github.com/ant31/crd-validation, and we need the openapi_generated.go in this PR for the generation. Therefore PTAL.

@gaocegege
Copy link
Member Author

/assign @ScorpioCPH @jlewi

@gaocegege
Copy link
Member Author

It is not urgent anymore since we know that it is not practical to use crd validation feature to validate all types in the CRD. I am switching to the way described in #561 (comment)

@jlewi
Copy link
Contributor

jlewi commented May 29, 2018

/lgtm
/approve

Looks like we might need to manually merge though since coveralls test is busted by all the generated code.

@gaocegege Is this ready to merge?

@gaocegege
Copy link
Member Author

I will try to add ignore for the coverage test.

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 30, 2018
@gaocegege
Copy link
Member Author

/retest

1 similar comment
@gaocegege
Copy link
Member Author

/retest

@gaocegege
Copy link
Member Author

PTAL again @jlewi

@jlewi
Copy link
Contributor

jlewi commented May 31, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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

@gaocegege
Copy link
Member Author

/hold

I prefer to merge #610 first since this PR may have conflicts with it.

@gaocegege
Copy link
Member Author

/hold cancel

I will fix the conflicts asap.

Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
Signed-off-by: Ce Gao <gaoce@caicloud.io>
@k8s-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@gaocegege
Copy link
Member Author

Rebasing upstream/master, thus no need to review again.

@k8s-ci-robot k8s-ci-robot merged commit cb18925 into kubeflow:master May 31, 2018
@gaocegege gaocegege deleted the openapi branch May 31, 2018 07:01
yph152 pushed a commit to yph152/tf-operator that referenced this pull request Jun 18, 2018
* hack: Add openapi-gen

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* api: Generate openapi model

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* linter_config: Add

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* openapi: Add k8s.io

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* .travis: Ignore openapi

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* dep: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>
jetmuffin pushed a commit to jetmuffin/tf-operator that referenced this pull request Jul 9, 2018
* hack: Add openapi-gen

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* api: Generate openapi model

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* linter_config: Add

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* openapi: Add k8s.io

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* .travis: Ignore openapi

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* dep: Update

Signed-off-by: Ce Gao <gaoce@caicloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants