For a given .proto file, gosnappi client and gRPC server in non-go language may not always be compatible

ashutshkumr commented 2 years ago

We have hit this issue in multiple instances. Here's the reason why:

Let's take an exampe of otg.proto. The package name is defined as package otg;.

When we generate a C++ server skeleton, without any modification (in package name), we end up generating gRPC skeletons where URLs look like /otg.Openapi/SetConfig
When we generate gosnappi using openapiart, we modify the package name to be used as snappi, moreover openapiart further appends a suffix resulting in package name snappipb resulting in URL that looks like /snappipb.Openapi/SetConfig

This results in an incompatible API call.

There are two solutions that we need to choose from:

Ask any implementor of otg.proto to use snappipb as package name (or any other package name that's relevant to a given project). That's what we currently do for ixia-c gRPC server. https://github.com/open-traffic-generator/otg-grpc/blob/main/do.sh#L30
- This may resolve the problem quickly but does not seem like a good solution (because how do we enforce this ?)
- e.g. in this case, we didn't enforce this for ixia-c protocol-engine and ultimately we noticed the incompatibility
Modify the generated gRPC skeleton in gosnappi to use original package name in URL. We could also put some other restriction.

ashutshkumr commented 2 years ago

@ajbalogh need your input on this. @jkristia maybe this affects you too.

ajbalogh commented 2 years ago

The otg.proto that is generated and pushed as part of the open-traffic-generator/models repo is considered to be the source of truth with the expectation that protobuf/grpc generation would use that artifact as the input. The package name is otg not snappi as the snappi name is just a UX package naming convention. The build process for snappi should use the otg artifact or generate the same named artifact so that there would be no conflict. Minor bugs would need to be fixed (copy files, message option extension number declaration). Am i missing anything?

ashutshkumr commented 2 years ago

@ajbalogh that's about it, most probably change will be limited to snappi repo only, so will move this issue to that repo.

alakendu commented 2 years ago

fix should available in https://github.com/open-traffic-generator/snappi/releases/tag/v0.6.16

open-traffic-generator / snappi

For a given .proto file, gosnappi client and gRPC server in non-go language may not always be compatible #137