Copilot commented on code in PR #105:
URL:
https://github.com/apache/skywalking-banyandb-java-client/pull/105#discussion_r2596300716
##########
src/main/proto/banyandb/v1/banyandb-measure.proto:
##########
@@ -156,13 +156,31 @@ message TopNRequest {
repeated string stages = 9;
}
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+ // name of the tag family
+ string name = 1;
+ // names of tags in the tag family
+ repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+ // the tag family specification
+ repeated TagFamilySpec tag_family_spec = 1;
+ // the field names
+ repeated string field_names = 2;
+}
+
// DataPointValue is the data point for writing. It only contains values.
message DataPointValue {
// timestamp is in the timeunit of milliseconds.
google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required
= true];
// the order of tag_families' items match the measure schema
Review Comment:
Duplicate comment lines. Line 179 says "the order of tag_families' items
match the measure schema" and line 180 says "the order of tag_families' items
match DataPointSpec". The old comment (line 179) should be removed, keeping
only the updated comment (line 180) that references DataPointSpec.
```suggestion
```
##########
src/main/proto/banyandb/v1/banyandb-measure.proto:
##########
@@ -156,13 +156,31 @@ message TopNRequest {
repeated string stages = 9;
}
+// TagFamilySpec defines the specification of a tag family.
+message TagFamilySpec {
+ // name of the tag family
+ string name = 1;
+ // names of tags in the tag family
+ repeated string tag_names = 2;
+}
+
+// DataPointSpec defines the specification of a data point.
+message DataPointSpec {
+ // the tag family specification
+ repeated TagFamilySpec tag_family_spec = 1;
+ // the field names
+ repeated string field_names = 2;
+}
+
// DataPointValue is the data point for writing. It only contains values.
message DataPointValue {
// timestamp is in the timeunit of milliseconds.
google.protobuf.Timestamp timestamp = 1 [(validate.rules).timestamp.required
= true];
// the order of tag_families' items match the measure schema
+ // the order of tag_families' items match DataPointSpec
repeated model.v1.TagFamilyForWrite tag_families = 2
[(validate.rules).repeated.min_items = 1];
// the order of fields match the measure schema
Review Comment:
Duplicate comment lines. Line 182 says "the order of fields match the
measure schema" and line 183 says "the order of fields match DataPointSpec".
The old comment (line 182) should be removed, keeping only the updated comment
(line 183) that references DataPointSpec.
```suggestion
```
##########
src/main/proto/banyandb/v1/banyandb-trace.proto:
##########
@@ -25,12 +25,16 @@ import "banyandb/v1/banyandb-model.proto";
option go_package =
"github.com/apache/skywalking-banyandb/api/proto/banyandb/trace/v1";
option java_package = "org.apache.skywalking.banyandb.trace.v1";
-// Write messages
+message TagSpec {
+ repeated string tag_names = 1;
+}
+
message WriteRequest {
common.v1.Metadata metadata = 1;
- repeated model.v1.TagValue tags = 2;
+ repeated model.v1.TagValue tags = 2 [(validate.rules).repeated.min_items =
1];
bytes span = 3;
- uint64 version = 4;
+ uint64 version = 4 [(validate.rules).uint64.gt = 0];
Review Comment:
Missing import for validation. The WriteRequest uses validation rules like
`[(validate.rules).repeated.min_items = 1]` and `[(validate.rules).uint64.gt =
0]`, but the file doesn't import "validate/validate.proto". Add `import
"validate/validate.proto";` after the other imports to enable validation rules.
##########
src/main/proto/banyandb/v1/banyandb-measure.proto:
##########
@@ -172,10 +190,16 @@ message DataPointValue {
message WriteRequest {
// the metadata is required.
common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
Review Comment:
Duplicate field definition for `metadata` with field number 1. Lines 192 and
194 both define `metadata = 1` with different comments and validation rules.
This will cause a compilation error in protobuf. The old definition (line 192)
should be removed, keeping only the new definition (line 194) without the
`required` validation rule.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
