-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Generate OpenAPI specification and Swagger file #3081
Conversation
|
Hi @jinchihe. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cc @vdemeester |
@jinchihe thanks for the PR 👼
I tend to agree, we could have the SDK in a separate repository (a bit like Overall, I think it look good, but I feel we should split the PR into several :
|
Would it make sense to have keep the python client lib generation as a separate project, maybe in experimental or a separate repo? @tektoncd/core-maintainers |
I would prefer that. Either as a directory in tektoncd/experimental, or just as a separately-owned repo outside the Tekton org, based on Tekton-owned generated Swagger. Basically, I definitely think Tekton should own Swagger doc generation, but I don't want to own generated Python (or any non-Go) client libraries since I don't think we can rely on sufficient Python expertise in case there are issues. |
@vdemeester Thank you for the comments. Make sense, we may can commit the openapi and swagger in |
@imjasonh why put outside the Tekton org? personally I suggest to maintain that inside of Tekton org, you know, that has close relationship with tekton/pipeline and very import for tekton end user. If you‘re worry about sufficient Python expertise, this is open source, I think if have problems, more and more Python expertise will take part in :-) |
Perhaps I should rephrase. 😄 I'd like to start with the generated Python client (and other generated clients) in a non-Tekton repo, or in Having generated clients means having a release process for them, publishing them to PyPi, etc., and I just don't think there's enough dedicated expertise or interest among Tekton owners and partners at this time to justify adding that burden. If the clients are a hit and are widely used, then presumably some dependent user would be willing to step and own generating and releasing new clients, and debugging the inevitable user issues. The best way to gauge use and interest is to start by publishing them -- anywhere -- and seeing how they get adopted. |
@imjasonh @vdemeester OK thanks. I will separate the PR to keep the openapi and swagger parts in this repo, and commit the generated python SDK in |
92f56be
to
a951c04
Compare
@imjasonh @vdemeester As above discussion, I keep the OpenAPI and swagger parts in the if so, the PR is ready for reviewing and testing. I will update summary and description. Thanks. |
a951c04
to
f2cb570
Compare
/ok-to-test |
/hold |
Hey sorry @jinchihe could you update the commit message to:
After that I think this is ready to go! |
/approve cancel |
f180105
to
fdf6431
Compare
The following is the coverage report on the affected files.
|
@sbwsg so sorry... I missed your comments, update later. I already updated that and removed SDK related message. And I noticed there are some changes in types, so I also re-generated the OpenAPI specification and Swagger file. Thanks a lot! |
fdf6431
to
46f1290
Compare
The following is the coverage report on the affected files.
|
46f1290
to
e98e1a7
Compare
The following is the coverage report on the affected files.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg 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 |
/hold cancel |
/lgtm |
Changes
The PR is going to generate OpenAPI specification, and generate Swagger file according to the OpenAPI specification.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Includes tests (if functionality changed/added):
Includes docs (if user facing)
Commit messages follow commit message best practices
Release notes block has been filled in or deleted (only if no user facing changes)
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmd
dir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes