-
Notifications
You must be signed in to change notification settings - Fork 4.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
Proposal: Check-in generated golang protos in envoy/api #8151
Comments
@nareddyt I get that this is the way that the Go proto ecosystem tends to work. Here is how I can see it being realistic for Envoy if you want this:
If we have all the above satisfied, I think this would be totally reasonable and I'm supportive. |
CC other @envoyproxy/api-shepherds |
I agree that the generated go protobufs need to be published somewhere to avoid bifurcation in go/envoy ecosystem. I was going to add a script to mirror generated go files from bazel to a repo (something like https://github.com/kythe/kythe/blob/master/kythe/proto/generate_go_protobufs.py). The question would remain about versioning scheme. I also had work in progress to move gogo ad-hoc script into bazel world. |
Thanks for the quick reply @htuch. Some of my thoughts:
|
I'm strongly in favor of the solution that @htuch proposes which is to mirror the Go protos on every master commit into a new repo. I think this should be pretty easy and would be great for the community. I know we would use this at Lyft and I was actually discussing this with @lita and @ryancox today. It's possible we could help out with this also. |
It should be possible to create a new branch in go-control-plane (v2?) to host the standard protos. The import path would be a bit strange, e.g. |
PR #8163 deleted all gogoproto annotations to help with separating go/gogo proto builds. |
Status Update:
@kyessenov: Should this issue be closed now, or will you close it after the CI change? |
Let's close this after we get CI auto-merge working. |
Adds a script to create a go module from the generated protobufs as part of #8151. The module appears to build with the following module declaration: module github.com/envoyproxy/data-plane-api/api go 1.12 require ( github.com/census-instrumentation/opencensus-proto v0.2.1 github.com/envoyproxy/protoc-gen-validate v0.1.0 github.com/gogo/protobuf v1.3.0 github.com/golang/protobuf v1.3.2 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 google.golang.org/grpc v1.23.0 ) Add CI automation to trigger the script after the merge to master in envoyproxy. Risk Level: low Testing: local build Docs Changes: none Release Notes: none Fixes #8151 Signed-off-by: Kuat Yessenov <kuat@google.com>
Adds a script to create a go module from the generated protobufs as part of envoyproxy#8151. The module appears to build with the following module declaration: module github.com/envoyproxy/data-plane-api/api go 1.12 require ( github.com/census-instrumentation/opencensus-proto v0.2.1 github.com/envoyproxy/protoc-gen-validate v0.1.0 github.com/gogo/protobuf v1.3.0 github.com/golang/protobuf v1.3.2 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 google.golang.org/grpc v1.23.0 ) Add CI automation to trigger the script after the merge to master in envoyproxy. Risk Level: low Testing: local build Docs Changes: none Release Notes: none Fixes envoyproxy#8151 Signed-off-by: Kuat Yessenov <kuat@google.com>
Adds a script to create a go module from the generated protobufs as part of envoyproxy#8151. The module appears to build with the following module declaration: module github.com/envoyproxy/data-plane-api/api go 1.12 require ( github.com/census-instrumentation/opencensus-proto v0.2.1 github.com/envoyproxy/protoc-gen-validate v0.1.0 github.com/gogo/protobuf v1.3.0 github.com/golang/protobuf v1.3.2 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 google.golang.org/grpc v1.23.0 ) Add CI automation to trigger the script after the merge to master in envoyproxy. Risk Level: low Testing: local build Docs Changes: none Release Notes: none Fixes envoyproxy#8151 Signed-off-by: Kuat Yessenov <kuat@google.com>
Adds a script to create a go module from the generated protobufs as part of envoyproxy#8151. The module appears to build with the following module declaration: module github.com/envoyproxy/data-plane-api/api go 1.12 require ( github.com/census-instrumentation/opencensus-proto v0.2.1 github.com/envoyproxy/protoc-gen-validate v0.1.0 github.com/gogo/protobuf v1.3.0 github.com/golang/protobuf v1.3.2 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 google.golang.org/grpc v1.23.0 ) Add CI automation to trigger the script after the merge to master in envoyproxy. Risk Level: low Testing: local build Docs Changes: none Release Notes: none Fixes envoyproxy#8151 Signed-off-by: Kuat Yessenov <kuat@google.com>
Hi envoy-api shepards,
Given the recent improvement to the golang APIs via #8003 and the upcoming goproto compatible version of go-control-plane via envoyproxy/go-control-plane#213, I wanted to propose another change: Is it possible that we could check in generate golang protos into the
api/
folder?Most golang code uses the built-in go build system instead of Bazel. This is a standard in the open-source golang community. This means that the generated golang protos (.pb.go files) must be checked into these repos, allowing consumers to depend upon them and build them using the go build system.
Currently, envoy's api repository does not have the generated golang protos. Instead, both go-control-plane/envoy and gprc-go/xds/internal/proto/envoy contain the generated envoy golang protos. This is a problem because:
Checking in generated files is OK for golang protos. Other than go-control-plane and grpc-go, some other repositories also follow the same structure for non-envoy protos:
It would be ideal to have golang protos auto-generated and checked-in to the
api/
folder whenever any of the underlying protos files are updated. Proto generation would occur withprotoc
using grpc and protoc-gen-validate.I know that envoy primarily supports C++, but this change would help drive golang adoption for the control plane.
The text was updated successfully, but these errors were encountered: