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

Building protobuf assets #1374

Closed
fsolleza opened this issue Feb 9, 2024 · 4 comments
Closed

Building protobuf assets #1374

fsolleza opened this issue Feb 9, 2024 · 4 comments
Labels
question Further information is requested

Comments

@fsolleza
Copy link
Contributor

fsolleza commented Feb 9, 2024

The protobuf assets for some services are in the src directory for that service (e.g., src/accountingservice/genproto) . Applying a change in pb/demo.proto requires copying this file or using protoc to generate the required files in each src/Xservice directory before invoking docker compose build... or similar (I think). By contrast, the other services' Dockerfiles handle the copy and build of protobuf assets.

The following services (all golang) need to compile pb/demo.proto using protoc into their respective directories before docker compose build...:

  1. accountingservice
  2. checkoutservice
  3. productcatalogservice

The following service (c++) requires copying pb/demo.proto to src/currencyservice/proto/demo.proto

  1. currencyservice

Is there a reason for this? I have a fork that fixes this behavior but thought I would check first.

Use Github Discussions.

@fsolleza fsolleza added the question Further information is requested label Feb 9, 2024
@puckpuck
Copy link
Contributor

puckpuck commented Feb 9, 2024

We have a make target that will generate the protobuf assets or copy the demo.proto file for all services as required. Each language has their own way on how to do this, so instead of needing to know all of them, we created a single script that does it for all fo them.

make generate-protobuf will set up protobuf assets for all services. If you don't have the proper tooling (ie: protoc) it will gracefully move to the next service attempting each one.

We took this approach to allow people to make simple code changes, and then do a docker compose build ... without needing to generate the protobuf assets first.

If you feel like there is a better way, and you developed one in your fork, I would be interested to learn more.

@fsolleza
Copy link
Contributor Author

fsolleza commented Feb 9, 2024

Thanks! Here's the scenario I was dealing with that led to my changes

  1. Make change to pb/demo.proto
  2. make start and realize that some things break
  3. make generate-protobuf and realize that I need tooling for the different builds: copy into directory for many services, protoc + plugins for golang, npm, etc.
  4. install all of them and build successfully

My setup makes changes to the Dockerfiles of the 4 services noted above so that a change in pb/demo.proto doesnt require all the different builds. Of course, this means that a new service for example, will need to build protobuf assets in their new Dockerfile, but at least they won't need to build all assets manually. This also doesn't take into account the fact that if a service uses a changed definition in pb/demo.proto, then that service would need some code updates.

Further, it makes the deployment a bit more consistent. At the moment, with a change in pb/demo.proto, the four services above need make generate-protobuf while others do not (along with all the tooling necessary).

This doesn't replace make generate-protobuf. It makes the make start and friends friendlier to small changes to pb/demo.proto.

The changes in my fork can be found here. Let me know if these are sensible changes.

dfaf53f

@puckpuck
Copy link
Contributor

I think you should submit a PR from that fork :D

We will likely need some new entries in .gitignore so that people who run make generate-protobuf don't have any changes that need to be committed.

@puckpuck
Copy link
Contributor

this was fixed with #1386

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

No branches or pull requests

2 participants