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

protoc-gen-openapi: add flag to generate source_relative yaml #359

Merged
merged 9 commits into from
Mar 24, 2023

Conversation

gfelbing
Copy link
Contributor

In Monorepos, merging all proto files into a single openapi.yaml isn't useful.
This PR adds a flag for separating generated openapi.yaml files for each proto file.

Fixes #268

@gfelbing gfelbing requested a review from a team as a code owner July 12, 2022 11:27
@google-cla
Copy link

google-cla bot commented Jul 12, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@TC-AVNP
Copy link

TC-AVNP commented Aug 30, 2022

Looking forward to this!

@maetad
Copy link

maetad commented Sep 22, 2022

Nice PR

@nightlyone
Copy link
Contributor

Hi @timburks could you please take a look and start a first round of review? That PR proved pretty useful, if you host multiple APIs in the same monorepo.

@timburks
Copy link
Contributor

timburks commented Oct 7, 2022

@gfelbing @nightlyone I'm open to merging this, but as I've mentioned elsewhere, this isn't how the APIs in github.com/googleapis/googleapis are structured (which I'm most familiar with). Could we get a simple set of protos (two or three) that illustrate how you're expecting this to work? (I'm guessing each .proto is a complete API). Then we can add a few tests to help us maintain this. Thanks!

@gfelbing
Copy link
Contributor Author

Hi @timburks !
Thanks for your answer. Indeed, each .proto is a complete API.
Use-cases are monorepos that are using buf for proto generation/linting, e.g. in the following structure:

├── buf.gen.yaml
├── buf.lock
├── buf.yaml
└── services
    ├── service-a
    │   └── api.proto
    └── service-b
        └── api.proto

With the following buf.gen.yaml:

version: v1
plugins:
  - name: openapi
    out: .
    opt:
      - output_mode=source_relative

buf generate should result in separate APIs for each service:

├── buf.gen.yaml
├── buf.lock
├── buf.yaml
└── services
    ├── service-a
    │   ├── api.openapi.yaml
    │   └── api.proto
    └── service-b
        ├── api.openapi.yaml
        └── api.proto

@gfelbing gfelbing force-pushed the protoc-gen-openapi-source-relative branch from 286f592 to 31ec2f7 Compare November 2, 2022 13:10
@gfelbing
Copy link
Contributor Author

gfelbing commented Nov 2, 2022

Hi @timburks !
Thanks for looking into this. As requested, I added testing for the source_relative flag.

I was able to reuse the existing test cases: If we run the generator with the source_relative option on all examples at once, we can assert equality to the result of the single execution.
I chose the fq_schema_naming test case on purpose, as most examples define a message "Message" which clashes when omitting the package name in the resulting openapi.yaml.

@li1234yun
Copy link

Watching 👍

@li1234yun
Copy link

li1234yun commented Dec 8, 2022

Hi, @timburks @gfelbing !

I have an idea about the pr, maybe you can consider same directory protobuf files generate one openapi file.

├── buf.gen.yaml
├── buf.lock
├── buf.yaml
└── services
    ├── service-a
    │   └── api1.proto
    |   |__ api2.proto
    └── service-b
        └── api.proto

=> use protoc-gen-openapi to generate openapi doc.

├── buf.gen.yaml
├── buf.lock
├── buf.yaml
└── services
    ├── service-a
    │   ├── api.openapi.yaml
    │   └── api1.proto
    |   └── api2.proto
    └── service-b
        ├── api.openapi.yaml
        └── api.proto

Maybe same directory proto files can generate one openapi yaml/json file.

Thank you for sharing so excellent tool!

@gfelbing gfelbing force-pushed the protoc-gen-openapi-source-relative branch from 03d73da to 422fa65 Compare December 23, 2022 08:32
@gfelbing
Copy link
Contributor Author

@li1234yun Thanks for your suggestion!
I see were you want to go and I think it could be a useful behavior.
Yet it would be different behavior than the default source_relative, which could be surprising for some users.
As this PR introduces an output_mode flag, your approach could be implemented with a different value like output_mode=by_folder.
I'd suggest to postpone this discussion to a follow-up PR, as this would blow the scope of this one which is already waiting quite some time here. Would that be okay for you?

@timburks thanks for updating the feature branch! I fixed the tests again, there were some c/p issues in the new test data.
This is actually quite nice that the source_relative feature detects issues like that :)
Do you see any more obstacles for this feature?

Copy link
Contributor

@timburks timburks left a comment

Choose a reason for hiding this comment

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

@gfelbing thanks for adding this and the extra tests! @li1234yun, I think the suggestion to support a different output mode seems workable considering how localized the changes would be in generator.go. Followup PRs would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

protoc-gen-openapi - incorrect output for multiple proto files
6 participants