openconfig / gnmi

gRPC Network Management Interface
Apache License 2.0
459 stars 196 forks source link

Container-based proto compilation #136

Closed n0shut closed 4 months ago

n0shut commented 1 year ago

Hi @robshakir @marcushines

I would like to propose a hermetic way to compile protos used in this repository which is based on the container approach.

Currently, the compile-protos.sh script has no pinned dependencies resulting in users defaulting to whatever latest/oldest version they have installed on their machines when updating the protos.

Not only it adds unnecessary churn to the PRs, it also makes the process overly complicated. The dev machine should have protos installed in the specified directories and we see that people struggle with that - #134.

In the newly added run.sh I introduced a container-based workflow where I leverage ghcr.io/srl-labs/protoc container that has pinned dependencies for protoc and the relevant grpc plugins - https://github.com/srl-labs/protoc-container/blob/main/Dockerfile

Using this container every user of gnmi repo will be guaranteed to compile the protos in one and only possible way, without the need to keep local dependencies and maintain their lifecycle.

At the time of this writing the latest version of protoc and plugins were used. I hope you will find that useful.

In #137 I generated the Go/Py bindings for you to verify how it works and potentially try it yourself to ensure that it works the same for everyone in a consistent way.

# to compile all protos
./run.sh compile-all

# to compile selected protos
./run.sh compile-go-gnmi-ext
n0shut commented 1 year ago

Hi @robshakir @wenovus I have updated this PR by introducing ./run.sh script that embeds purpose-built functions for each lang/proto. I have updated the initial PR comment to reflect that, but in essense what you can now do is:

# to compile all protos
./run.sh compile-all

# to compile selected protos
./run.sh compile-go-gnmi-ext
hellt commented 7 months ago

tsk tsk tsk still using this user unfriendly compile_protos.sh that has no reproducibility and heavily dependent on hosts' configuration

robshakir commented 7 months ago

Thanks for this -- let me look again this week. Unfortunately, I was on an extended leave during the period you previously requested review.

hellt commented 7 months ago

No worries @robshakir, I didn't mean to push, just a slight nudge. I can update it with the updated versions of protoc and go plugins

robshakir commented 7 months ago

@marcushines - PTAL too.

n0shut commented 7 months ago

compile_protos.sh brought back, @robshakir

n0shut commented 4 months ago

closing as stale