hanahmily commented on code in PR #322:
URL:
https://github.com/apache/skywalking-banyandb/pull/322#discussion_r1323859297
##########
api/proto/banyandb/stream/v1/write.proto:
##########
@@ -39,13 +39,28 @@ message ElementValue {
}
message WriteRequest {
- // the metadata is only required in the first write.
+ // the metadata is required.
common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
// the element is required.
ElementValue element = 2 [(validate.rules).message.required = true];
}
-message WriteResponse {}
+// Status is the response status for write
+enum Status {
+ STATUS_UNSPECIFIED = 0;
+ STATUS_SUCCEED = 1;
+ STATUS_RECEIVE_ERROR = 2;
+ STATUS_INVALID_TIMESTAMP = 3;
+ STATUS_INVALID_METADATA = 4;
+ STATUS_EXPIRED_REVISION = 5;
+ STATUS_INTERNAL_ERROR = 6;
+}
+
+message WriteResponse {
Review Comment:
The same as the measure's suggestions.
##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -45,8 +45,23 @@ message WriteRequest {
DataPointValue data_point = 2 [(validate.rules).message.required = true];
}
+// Status is the response status for write
+enum Status {
+ STATUS_UNSPECIFIED = 0;
+ STATUS_SUCCEED = 1;
+ STATUS_RECEIVE_ERROR = 2;
+ STATUS_INVALID_TIMESTAMP = 3;
+ STATUS_INVALID_METADATA = 4;
+ STATUS_EXPIRED_REVISION = 5;
+ STATUS_INTERNAL_ERROR = 6;
+}
+
// WriteResponse is the response contract for write
-message WriteResponse {}
+message WriteResponse {
+ common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
Review Comment:
Metadata should only be returned in case the status is
STATUS_INVALID_METADATA.
##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -45,8 +45,23 @@ message WriteRequest {
DataPointValue data_point = 2 [(validate.rules).message.required = true];
}
+// Status is the response status for write
+enum Status {
+ STATUS_UNSPECIFIED = 0;
+ STATUS_SUCCEED = 1;
+ STATUS_RECEIVE_ERROR = 2;
+ STATUS_INVALID_TIMESTAMP = 3;
+ STATUS_INVALID_METADATA = 4;
+ STATUS_EXPIRED_REVISION = 5;
+ STATUS_INTERNAL_ERROR = 6;
+}
+
// WriteResponse is the response contract for write
-message WriteResponse {}
+message WriteResponse {
Review Comment:
I would like to define request identification to both request and response.
You can add a unique messageId to both the WriteRequest and WriteResponse
messages. With this messageId, we can easily identify which response
corresponds to a particular request.
##########
banyand/liaison/grpc/discovery.go:
##########
@@ -223,22 +223,25 @@ var _ schema.EventHandler = (*entityRepo)(nil)
type entityRepo struct {
log *logger.Logger
- entitiesMap map[identity]partition.EntityLocator
+ entitiesMap map[identity]*partition.EntityLocator
Review Comment:
```suggestion
entitiesMap map[identity]partition.EntityLocator
```
The receiver of methods defined in EntityLocator is the value instead of a
pointer.
##########
api/proto/banyandb/measure/v1/write.proto:
##########
@@ -45,8 +45,23 @@ message WriteRequest {
DataPointValue data_point = 2 [(validate.rules).message.required = true];
}
+// Status is the response status for write
+enum Status {
+ STATUS_UNSPECIFIED = 0;
+ STATUS_SUCCEED = 1;
+ STATUS_RECEIVE_ERROR = 2;
+ STATUS_INVALID_TIMESTAMP = 3;
+ STATUS_INVALID_METADATA = 4;
+ STATUS_EXPIRED_REVISION = 5;
+ STATUS_INTERNAL_ERROR = 6;
+}
+
// WriteResponse is the response contract for write
-message WriteResponse {}
+message WriteResponse {
+ common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
Review Comment:
The name should be "latestMetadata" which hints it's the most recent
revision.
##########
api/proto/banyandb/stream/v1/write.proto:
##########
@@ -39,13 +39,28 @@ message ElementValue {
}
message WriteRequest {
- // the metadata is only required in the first write.
+ // the metadata is required.
common.v1.Metadata metadata = 1 [(validate.rules).message.required = true];
// the element is required.
ElementValue element = 2 [(validate.rules).message.required = true];
}
-message WriteResponse {}
+// Status is the response status for write
+enum Status {
Review Comment:
Please move it and stream's Status into "model" package
--
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]
