You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Hi @laminelam , will this change be needed in the REST side? If it's only for the gRPC side, I wonder if we should consider moving this to the opensearch-protobufs repository instead.
Just some preliminary thoughts on placement in protobufs repo: I'm thinking it can be added manually to common.proto for now, which the protobuf tooling to autogenerate protobufs should not overwrite (not until the next OpenSearch 4.0 major release at least). Meanwhile, the protobuf maintainers can look for a more long term solution to support fields specific only to only in the protobufs and not the REST API in the future. @lucy66hw@laminelam WDYT?
Hi @laminelam , will this change be needed in the REST side? If it's only for the gRPC side, I wonder if we should consider moving this to the opensearch-protobufs repository instead.
Just some preliminary thoughts on placement in protobufs repo: I'm thinking it can be added manually to common.proto for now, which the protobuf tooling to autogenerate protobufs should not overwrite (not until the next OpenSearch 4.0 major release at least). Meanwhile, the protobuf maintainers can look for a more long term solution to support fields specific only to only in the protobufs and not the REST API in the future. @lucy66hw@laminelam WDYT?
Hi @karenyrx
I am fine to manually add the new protos directly to protobufs repo. We need to find a way to prevent it from being overridden by the auto-generating process.
I am fine to manually add the new protos directly to protobufs repo. We need to find a way to prevent it from being overridden by the auto-generating process.
The tooling will not overwrite any protobufs, if you add an annotation next to the field [(tooling_skip) = true];, e.g.:
@laminelam Let's port this PR over to opensearch-protobufs then?
Thank you @karenyrx for adding this new feature
Here's the protobufs PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introducing new proto messages to support indexing of primitive arrays in gRPC outside of source.
Related issue: opensearch-project/OpenSearch#19638
For now, only float arrays are supported, as well as BytesValue.
Two options are offered:
This is the expected proto messages
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.