search

opensearch-project / opensearch-api-specification

API specification for OpenSearch
Apache License 2.0
33 stars 61 forks source link

[PROPOSAL] Publish protobuf files. #532

Open dblock opened 2 months ago

dblock commented 2 months ago

What/Why

What are you proposing?

Coming from https://github.com/opensearch-project/OpenSearch/pull/15411 and https://github.com/opensearch-project/OpenSearch/issues/15190 I propose we generate and publish protobuf files as part of the build/release workflow.

What users have asked for this feature?

OpenSearch is trying to use the OpenAPI spec to generate .proto files then use those on both client and server sides.

What problems are you trying to solve?

OpenSearch server wants .proto files. OpenSearch clients that will implement gRPC/protobuf want .proto files.

What is the developer experience going to be?

A release of opensearch-api-specification will contain a .zip with .proto files.

Why should it be built? Any reason not to?

The right place for .proto files is this repo because multiple consumers want .proto files.

dblock commented 2 months ago

cc: @andrross @amberzsy

philiplhchan commented 2 months ago

I think currently one challenge for an automated release pipeline is how we're going to ensure that the generated proto definitions are backwards compatible, e.g. if we add a new field/type in the API spec, the new generated proto should not change any of the field tags, type names, etc. of any of the existing definitions because protobuf doesn't allow that.

  1. Ideally the transpiler should be stable (e.g. it wouldn't work if the tag IDs are generated randomly on each execution)
  2. we'd need some automated check to ensure no previously defined/published types/fields are modified, otherwise it would break the API for any clusters/clients running different versions.
dblock commented 2 months ago

@philiplhchan I think you're saying we need to generate proto files for a given API version. This can be done by first generating a version specific API (npm run merge -- --opensearch-version=2.16, see doc). What this does is that it removes everything per semver. Then you can covert that to .proto files. Right now the main pre-release is a moving target as we're fixing the spec quickly, but obviously a real release could follow this.

Regardless, consumers of the spec should not rely on a moving target. Commit the output and update it via PRs so you can catch anything breaking.

dblock commented 2 months ago

a) can we follow same format here? easier to parsing.

Yes. We would want a linter to fail when something inconsistent is used, also @nhtruong might have an opinion here.

dblock commented 2 months ago

Should we want to group into few proto files?

To me what's important is to organize the source code that is written by humans well. But these are machine generated so I wouldn't, it's less work and easier to grok changes over smaller files than merged ones.