From 9cb9445155804fb65af100b9747d467254fb7ca6 Mon Sep 17 00:00:00 2001
From: Yuchen Zeng <zyc@google.com>
Date: Wed, 20 Jul 2016 16:39:31 -0700
Subject: [PATCH] Addressed review comments
---
doc/command_line_tool.md | 29 +++++++++++++------
test/cpp/util/grpc_cli.cc | 13 +++++----
test/cpp/util/proto_file_parser.cc | 2 +-
.../proto_reflection_descriptor_database.cc | 4 +--
4 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/doc/command_line_tool.md b/doc/command_line_tool.md
index 89a70548b8..79a131c041 100644
--- a/doc/command_line_tool.md
+++ b/doc/command_line_tool.md
@@ -15,6 +15,7 @@ The command line tool can do the following things:
- Send unary rpc.
- Attach metadata and display received metadata.
- Handle common authentication to server.
+- Infer request/response types from server reflection result.
- Find the request/response types from a given proto file.
- Read proto request in text form.
- Read request in wire form (for protobuf messages, this means serialized binary form).
@@ -24,7 +25,6 @@ The command line tool can do the following things:
The command line tool should support the following things:
- List server services and methods through server reflection.
-- Infer request/response types from server reflection result.
- Fine-grained auth control (such as, use this oauth token to talk to the server).
- Send streaming rpc.
@@ -46,24 +46,35 @@ https://github.com/grpc/grpc/blob/master/test/cpp/util/grpc_cli.cc
Send a rpc to a helloworld server at `localhost:50051`:
```
-$ bins/opt/grpc_cli call localhost:50051 SayHello examples/protos/helloworld.proto \
- "name: 'world'" --enable_ssl=false
+$ bins/opt/grpc_cli call localhost:50051 SayHello "name: 'world'" \
+ --enable_ssl=false
```
On success, the tool will print out
```
Rpc succeeded with OK status
-Response:
+Response:
message: "Hello world"
```
The `localhost:50051` part indicates the server you are connecting to. `SayHello` is (part of) the
-gRPC method string. Then there is the path to the proto file containing the service definition,
-if it is not under current directory, you can use `--proto_path` to specify a new search root.
-`"name: 'world'"` is the text format of the request proto message.
-We are not using ssl here by `--enable_ssl=false`. For information on more
-flags, look at the comments of `grpc_cli.cc`.
+gRPC method string. Then `"name: 'world'"` is the text format of the request proto message. We are
+not using ssl here by `--enable_ssl=false`. For information on more flags, look at the comments of `grpc_cli.cc`.
+
+### Use local proto files
+
+If the server does not have the server reflection service, you will need to provide local proto
+files containing the service definition. The tool will try to find request/response types from
+them.
+
+```
+$ bins/opt/grpc_cli call localhost:50051 SayHello "name: 'world'" \
+ --protofiles=examples/protos/helloworld.proto --enable_ssl=false
+```
+
+If the proto files is not under current directory, you can use `--proto_path` to specify a new
+search root.
### Send non-proto rpc
diff --git a/test/cpp/util/grpc_cli.cc b/test/cpp/util/grpc_cli.cc
index fdb1a7c2a0..53529da782 100644
--- a/test/cpp/util/grpc_cli.cc
+++ b/test/cpp/util/grpc_cli.cc
@@ -35,14 +35,14 @@
A command line tool to talk to a grpc server.
Example of talking to grpc interop server:
grpc_cli call localhost:50051 UnaryCall "response_size:10" \
- --proto_file=src/proto/grpc/testing/test.proto --enable_ssl=false
+ --protofiles=src/proto/grpc/testing/test.proto --enable_ssl=false
Options:
- 1. --proto_file, use this flag to provide a proto file if the server does
+ 1. --protofiles, use this flag to provide a proto file if the server does
does not have the reflection service.
2. --proto_path, if your proto file is not under current working directory,
use this flag to provide a search root. It should work similar to the
- counterpart in protoc. This option is valid only when proto_file is
+ counterpart in protoc. This option is valid only when protofiles is
provided.
3. --metadata specifies metadata to be sent to the server, such as:
--metadata="MyHeaderKey1:Value1:MyHeaderKey2:Value2"
@@ -90,7 +90,8 @@ DEFINE_string(output_binary_file, "",
DEFINE_string(metadata, "",
"Metadata to send to server, in the form of key1:val1:key2:val2");
DEFINE_string(proto_path, ".", "Path to look for the proto file.");
-DEFINE_string(proto_file, "", "Name of the proto file.");
+// TODO(zyc): support a list of input proto files
+DEFINE_string(protofiles, "", "Name of the proto file.");
void ParseMetadataFlag(
std::multimap<grpc::string, grpc::string>* client_metadata) {
@@ -173,9 +174,9 @@ int main(int argc, char** argv) {
}
if (!request_text.empty()) {
- if (!FLAGS_proto_file.empty()) {
+ if (!FLAGS_protofiles.empty()) {
parser.reset(new grpc::testing::ProtoFileParser(
- FLAGS_proto_path, FLAGS_proto_file, method_name));
+ FLAGS_proto_path, FLAGS_protofiles, method_name));
} else {
parser.reset(new grpc::testing::ProtoFileParser(channel, method_name));
}
diff --git a/test/cpp/util/proto_file_parser.cc b/test/cpp/util/proto_file_parser.cc
index b1bf0471e1..5b0d925e1c 100644
--- a/test/cpp/util/proto_file_parser.cc
+++ b/test/cpp/util/proto_file_parser.cc
@@ -112,7 +112,7 @@ ProtoFileParser::ProtoFileParser(std::shared_ptr<grpc::Channel> channel,
LogError(
"Failed to get services from the server, "
"it may not have the reflection service.\n"
- "Please try to use the --proto_file option to provide a proto file.");
+ "Please try to use the --protofiles option to provide a proto file.");
}
if (has_error_) {
return;
diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc
index 48998551a5..2d847012a2 100644
--- a/test/cpp/util/proto_reflection_descriptor_database.cc
+++ b/test/cpp/util/proto_reflection_descriptor_database.cc
@@ -54,8 +54,8 @@ ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase(
: stub_(ServerReflection::NewStub(channel)) {}
ProtoReflectionDescriptorDatabase::~ProtoReflectionDescriptorDatabase() {
- if (!stream_) {
- GetStream()->WritesDone();
+ if (stream_) {
+ stream_->WritesDone();
Status status = stream_->Finish();
if (!status.ok()) {
gpr_log(GPR_ERROR,
--
GitLab