-
Notifications
You must be signed in to change notification settings - Fork 254
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
Support buf build deps #337
Comments
I would also like to have this, though as far as I know https://buf.build/googleapis/googleapis is actually maintained by buf themselves, and not google. I'm not sure if google has, or ever will, provide stuff on buf. Though, given that @timburks gave your request a thumbs up, I remain hopeful that this might actually happen. :) |
I've been aware of buf but not using it. I'm not surprised that the googleapis repo on buf is maintained by buf directly, but I've been curious about buf so this seems like a good opportunity to try it. I've signed into buf and found that I could create a gnostic organization. Looking at googleapis as an example, it seems that we would want to have a "gnostic" repo with subdirectories for "openapiv3", etc. I've uploaded a draft repo to buf.build/gnostic/gnostic. Please take a look and, if you're a buf user, please try it and comment here. To pass the buf linter, I made some changes to file names and paths that you can review at timburks/gnostic/tree/buf/buf. |
I think the change in the package name is good (as I think it follows proto naming conventions more closely), though it means that we can't use those options with protoc-gen-openapi anymore. Is your intention that these would be the new package names going forward and used everywhere? |
I add deps in # buf.yaml
deps:
- buf.build/gnostic/gnostic and # buf.gen.yaml
version: v1beta1
plugins:
- name: openapi
out: gen/openapi
strategy: all
opt:
- title=Openapiv3 Add the relevant definitions to the proto file import "gnostic/openapi/v3/annotations.proto";
option (gnostic.openapi.v3.document) = {
info: {
title: "Title from annotation";
version: "Version from annotation";
description: "Description from annotation";
contact: {
name: "Contact Name";
url: "https://github.com/google/gnostic";
email: "[email protected]";
}
license: {
name: "Apache License";
url: "https://github.com/google/gnostic/blob/master/LICENSE";
}
}
components: {
security_schemes: {
additional_properties: [
{
name: "BasicAuth";
value: {
security_scheme: {
type: "http";
scheme: "basic";
}
}
}
]
}
}
};
Run the |
@renyijiu its because protoc-gen-openapi is built against a different version of the protos which use a different package name. Right now you can't use the ones on buf.build with protoc-gen-openapi...unless you build your own version of it using the protos on buf.build. For now I've just copied the OpenApiv3 protos and annotations into its own local buf module. |
I also can't use proto-gen-openapi to generate openapi.v3.document property in the openapi.yaml. Why is this?@jeffsawatzky @renyijiu |
I'd like to move this repo to the proto structure in buf, or something similar that's better aligned with common practice for larger-scale proto-based projects. Because I've passed information about the new repo to the |
I just tried this and really love it. Could
|
This is largely working on my end with the buf.build repo, thanks @timburks However, since it hasn't been updated in a while it doesn't have this fix, which is causing issues for me. #344 any chance the fork can be updated with the What would it take to get this to a more mature state? My team is starting to get into Buf pretty heavily, and generating OpenAPI v3 schemas would be really helpful. |
@timburks any chance you can push the latest updates to https://buf.build/gnostic/gnostic? Similar to the previous comment, I believe I'm running into an issue fixed by #344. When I execute
The correct import is
I am getting around this currently by sed replacing the import post generation. |
@timburks I noticed that when we install
We can use the proto definitions from buf, the ones that use However, looking at this repo, it is still using the EDIT: My best guess is that since we are using buf to generate the openapi, and buf uses the buf.build definitions to pass to protoc, that protoc is able to parse the proto file using the buf package names, and when the openapi plugin gets the input it is already pre-parsed and uses something other than the package name to look up the extensions...like the field id perhaps? In other words, here where it looks up the extensions: |
@jeffsawatzky I don't recall making any special builds. I think these other definitions are working because their field numbers and types are the same, so deserializing with those protos works the same as the originals. AFAIK, the package name isn't used anywhere in the serialized protos (i.e. the binaries often stored as |
Catching up (and being dense), assuming the go_package change in the two protos in #344 is what we need to push to buf... |
Yeah I've been using tx7do's bsr instead and it works fine: https://buf.build/tx7do/gnostic/file/main:gnostic/openapi/v3/openapiv3.proto#L45 |
How's this? https://buf.build/timburks/gnostic-test If this looks good I'll push it to gnostic/gnostic |
I'll take that as LGTM - pushed to https://buf.build/gnostic/gnostic. All four protos were updated. |
Sorry to dig this out -- I've been working with, and noticed reflection does not work in my buf setup (mostly using https://github.com/SchwarzIT/go-template): $ grpcurl -plaintext -vv localhost:9090 list api.v1.ApiService
Failed to list methods for service "api.v1.ApiService": Symbol not found: api.v1.ApiService
caused by: File not found: gnostic/openapi/v3/annotations.proto after some debugging, I found out that the file needs to be imported with However, in the protobuf reflection API, it registers itself as protoregistry.GlobalFiles.FindFileByPath("openapiv3/annotations.proto") # works
protoregistry.GlobalFiles.FindFileByPath("gnostic/openapi/v3/annotations.proto") # worksn't 🙁 This seems far-fetched, but maybe we should align these import paths? It's a bit unfortunate, since I also haven't found any way yet to re-map them manually as a workaround. |
Thanks, @cfstras, this helped me debug the issue where my gRPC client was unable to connect to my server using reflect. I published the OpenAPI specs under the correct path in my buf repo ( |
As another workaround, I figured I can keep using gnostic if I omit the |
I tried it again and now openapi.yaml is generated correctly, with fields such as |
I have the same problem no matter what i do i keep getting Service not found: havi.hermes.v1.TemplateService Can you share your workaround ? |
I just don't |
In the example code of
protoc-gen-openapi
, I see thatoption (openapi.v3.documentation)
is already supported, but when using it in the proto file of other projects, the user needs to manually introduceimport "openapiv3/annotations.proto";
associated annotations.proto file to be able to compile successfully. Can you consider uploading it to buf.build, similar to https://buf.build/googleapis/googleapis, to make it easier for other users to introduce dependencies for use, thanks to this great project.The text was updated successfully, but these errors were encountered: