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

Supporting duplicate models (different packages) in protoc-gen-openapi #309

Closed
SoftMemes opened this issue Feb 14, 2022 · 5 comments · Fixed by #324
Closed

Supporting duplicate models (different packages) in protoc-gen-openapi #309

SoftMemes opened this issue Feb 14, 2022 · 5 comments · Fixed by #324

Comments

@SoftMemes
Copy link

I have a setup where the same model name is used in different packages (or different versions of the same package). In my protos, these are in separate directories (/foo/v1/bar.proto and /foo/v2/bar.proto, both declaring Bar).

The openapi (v2) generator from grpc-gateway handles this by including the version or if needed full package name in the generated model types, whereas with gnostic, I get duplicate types and errors in later tooling using the openapi spec.

What I would ideally want is to get the same style output in JavaScript based on the openapi spec as I do with other tooling directly using the grpc/proto definitions, with models keeping their names but packaged up in directories matching the proto package / directory structure, though I'm not sure if there's a way of representing this in openapi.

Is there a way of either getting the gnostic protoc-gen-openapi tool to work with this sort of setup, or alternatively, best practices for grpc/proto that means that this would not be a problem?

@morphar
Copy link
Collaborator

morphar commented Feb 14, 2022

I don't know if this is an option for you, but I also have a versioned API and I just generate seperately for v1 and v2.
I have my protos separated both "physically" (./v1/endpoint and ./v2/endpoint) and by package name in proto files (package flowstack.api.accounts.v1; and package flowstack.api.accounts.v2;).

@SoftMemes
Copy link
Author

Thank you for the suggestion. We follow the same naming (package and directory) but in this case, the aim would be to have a monorepo for all service contracts creating a single library (per language) used in other services. As individual services can be versioned separately, there wouldn't be a clean way to generate a v2 of the whole thing.

@morphar
Copy link
Collaborator

morphar commented Feb 14, 2022

I ran into that issue as well and decided to rethink my architecture, based on what tools I had available.
gnostic was the best fit for what I wanted and I decided to re-architect everything around how gnostic works.
The main thing I changed was the way I do versioning: instead of having v2 use v1 components, etc. I decided to make a clean cut between v1 and v2, so anything that v2 uses, that are in v1 is copied to v2.

I know that is the not necessarily the most desirable way of doing thing. E.g. Envoy Proxy makes heavy use of component usage between versions, but they mostly need documentation for proto usage.

I'm pretty sure it would be possible to get gnostic's protoc-gen-openapi to do something along the lines of what grpc-gateway does.
If you have the time and energy, I would suggest that you try and make the changes you need in a pull request.
Personally I can't spare the time for a change like this currently.

@shenqidebaozi
Copy link
Contributor

@morphar We have also encountered this problem recently. It is normal to use the same message name in different packages, but Gnostic does not seem to support different packages using the same message name. Should we splice the package name before the message name, such as pkg: for message: bar schema name: Foo_Bar

@morphar
Copy link
Collaborator

morphar commented Mar 16, 2022

@shenqidebaozi I don't think I understand the problem.
In which case are you unable to use the same name?
If you can create example proto files and what you do to re-create the problem, that would probably help me understand the problem better :)

jeffsawatzky added a commit to jeffsawatzky/gnostic that referenced this issue Mar 28, 2022
timburks pushed a commit that referenced this issue Mar 30, 2022
…ions for File/Document and Method/Operation (#324)

* support fully qualified schema names. fixes #309
* add support for file/document and method/operation annotations. fixes #308
* fix the package name in my tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants