[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #298: [WIP] RATIS-1111. Change the FileStore example to use Streaming

[GitBox <gi...@apache.org>]

szetszwo commented on a change in pull request #298:
URL: https://github.com/apache/incubator-ratis/pull/298#discussion_r530042599



##########
File path: ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreClient.java
##########
@@ -141,6 +155,14 @@ public long write(String path, long offset, boolean close, ByteBuffer buffer)
     return WriteReplyProto.parseFrom(reply).getLength();
   }
 
+  public DataStreamOutput getStreamOutput(String path) {
+    final StreamWriteRequestHeaderProto header = StreamWriteRequestHeaderProto.newBuilder()
+        .setPath(ProtoUtils.toByteString(path))
+        .build();
+    RaftClientRequest request = client.createDataStreamHeaderRequest(Message.valueOf(header.toByteString()));
+    return client.getDataStreamApi().stream(request);

Review comment:
       FileStoreClient should not create RaftClientRequest by itself. It will be automatically created.  Just send the path.
   ```
       return client.getDataStreamApi().stream(ProtoUtils.toByteString(path).asReadOnlyByteBuffer());
   ```

##########
File path: ratis-client/src/main/java/org/apache/ratis/client/api/DataStreamApi.java
##########
@@ -37,4 +39,6 @@
 public interface DataStreamApi {
   /** Create a stream to write data. */
   DataStreamOutput stream();
+
+  DataStreamOutput stream(RaftClientRequest request);

Review comment:
       Same as RaftClient, RaftClientRequest is for internal use only in Ratis. Please don't add this. (Similar to AsyncApi, the RaftClientRequest is for internal use only.)

##########
File path: ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java
##########
@@ -68,6 +68,8 @@
   /** Send set configuration request to the raft service. */
   RaftClientReply setConfiguration(RaftPeer[] serversInNewConf) throws IOException;
 
+  RaftClientRequest createDataStreamHeaderRequest(Message message);
+

Review comment:
       DataStream header is for internal use only in Ratis.  Please don't add this.  (Similar to AsyncApi, the RaftClientRequest is for internal use only.)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org