Revert "Add timeout flag to gRPC cli"

CMakeLists.txt

@@ -10079,7 +10079,6 @@ add_executable(cli_call_test
   test/cpp/util/cli_call.cc
   test/cpp/util/cli_call_test.cc
   test/cpp/util/cli_credentials.cc
-  test/cpp/util/cli_flags.cc
   test/cpp/util/grpc_tool.cc
   test/cpp/util/proto_file_parser.cc
   test/cpp/util/proto_reflection_descriptor_database.cc
@@ -11188,7 +11187,6 @@ add_executable(grpc_cli
   ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/reflection/v1alpha/reflection.grpc.pb.h
   test/cpp/util/cli_call.cc
   test/cpp/util/cli_credentials.cc
-  test/cpp/util/cli_flags.cc
   test/cpp/util/grpc_cli.cc
   test/cpp/util/grpc_tool.cc
   test/cpp/util/proto_file_parser.cc
@@ -11592,7 +11590,6 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
     ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h
     test/cpp/util/cli_call.cc
     test/cpp/util/cli_credentials.cc
-    test/cpp/util/cli_flags.cc
     test/cpp/util/grpc_tool.cc
     test/cpp/util/grpc_tool_test.cc
     test/cpp/util/proto_file_parser.cc

build_autogenerated.yaml

@@ -5416,7 +5416,6 @@ targets:
   headers:
   - test/cpp/util/cli_call.h
   - test/cpp/util/cli_credentials.h
-  - test/cpp/util/cli_flags.h
   - test/cpp/util/config_grpc_cli.h
   - test/cpp/util/grpc_tool.h
   - test/cpp/util/proto_file_parser.h
@@ -5430,7 +5429,6 @@ targets:
   - test/cpp/util/cli_call.cc
   - test/cpp/util/cli_call_test.cc
   - test/cpp/util/cli_credentials.cc
-  - test/cpp/util/cli_flags.cc
   - test/cpp/util/grpc_tool.cc
   - test/cpp/util/proto_file_parser.cc
   - test/cpp/util/proto_reflection_descriptor_database.cc
@@ -5871,7 +5869,6 @@ targets:
   headers:
   - test/cpp/util/cli_call.h
   - test/cpp/util/cli_credentials.h
-  - test/cpp/util/cli_flags.h
   - test/cpp/util/config_grpc_cli.h
   - test/cpp/util/grpc_tool.h
   - test/cpp/util/proto_file_parser.h
@@ -5881,7 +5878,6 @@ targets:
   - src/proto/grpc/reflection/v1alpha/reflection.proto
   - test/cpp/util/cli_call.cc
   - test/cpp/util/cli_credentials.cc
-  - test/cpp/util/cli_flags.cc
   - test/cpp/util/grpc_cli.cc
   - test/cpp/util/grpc_tool.cc
   - test/cpp/util/proto_file_parser.cc
@@ -5990,7 +5986,6 @@ targets:
   headers:
   - test/cpp/util/cli_call.h
   - test/cpp/util/cli_credentials.h
-  - test/cpp/util/cli_flags.h
   - test/cpp/util/config_grpc_cli.h
   - test/cpp/util/grpc_tool.h
   - test/cpp/util/proto_file_parser.h
@@ -6002,7 +5997,6 @@ targets:
   - src/proto/grpc/testing/simple_messages.proto
   - test/cpp/util/cli_call.cc
   - test/cpp/util/cli_credentials.cc
-  - test/cpp/util/cli_flags.cc
   - test/cpp/util/grpc_tool.cc
   - test/cpp/util/grpc_tool_test.cc
   - test/cpp/util/proto_file_parser.cc

src/proto/grpc/testing/echo.proto

@@ -15,17 +15,15 @@
 
 syntax = "proto3";
 
-package grpc.testing;
-
 import "src/proto/grpc/testing/echo_messages.proto";
 import "src/proto/grpc/testing/simple_messages.proto";
 
+package grpc.testing;
+
 service EchoTestService {
   rpc Echo(EchoRequest) returns (EchoResponse);
   rpc Echo1(EchoRequest) returns (EchoResponse);
   rpc Echo2(EchoRequest) returns (EchoResponse);
-  rpc CheckDeadlineUpperBound(SimpleRequest) returns (StringValue);
-  rpc CheckDeadlineSet(SimpleRequest) returns (StringValue);
   // A service which checks that the initial metadata sent over contains some
   // expected key value pair
   rpc CheckClientInitialMetadata(SimpleRequest) returns (SimpleResponse);
@@ -66,4 +64,5 @@ service UnimplementedEchoService {
 }
 
 // A service without any rpc defined to test coverage.
-service NoRpcService {}
+service NoRpcService {
+}

src/proto/grpc/testing/simple_messages.proto

@@ -17,10 +17,8 @@ syntax = "proto3";
 
 package grpc.testing;
 
-message SimpleRequest {}
+message SimpleRequest {
-
+}
-message SimpleResponse {}
 
-message StringValue {
+message SimpleResponse {
-  string message = 1;
 }

test/cpp/util/BUILD

@@ -122,14 +122,12 @@ grpc_cc_library(
     srcs = [
         "cli_call.cc",
         "cli_credentials.cc",
-        "cli_flags.cc",
         "proto_file_parser.cc",
         "service_describer.cc",
     ],
     hdrs = [
         "cli_call.h",
         "cli_credentials.h",
-        "cli_flags.h",
         "config_grpc_cli.h",
         "proto_file_parser.h",
         "service_describer.h",

test/cpp/util/cli_call.cc

@@ -18,16 +18,15 @@
 
 #include "test/cpp/util/cli_call.h"
 
+#include <iostream>
+#include <utility>
+
 #include <grpc/grpc.h>
 #include <grpc/slice.h>
 #include <grpc/support/log.h>
 #include <grpcpp/channel.h>
 #include <grpcpp/client_context.h>
 #include <grpcpp/support/byte_buffer.h>
-#include <math.h>
-
-#include <iostream>
-#include <utility>
 
 namespace grpc {
 namespace testing {
@@ -62,18 +61,6 @@ CliCall::CliCall(const std::shared_ptr<grpc::Channel>& channel,
       ctx_.AddMetadata(iter->first, iter->second);
     }
   }
-
-  // Set deadline if timeout > 0 (default value -1 if no timeout specified)
-  if (FLAGS_timeout > 0) {
-    int64_t timeout_in_ns = ceil(FLAGS_timeout * 1e9);
-
-    // Convert timeout (in microseconds) to a deadline
-    auto deadline =
-        gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC),
-                     gpr_time_from_nanos(timeout_in_ns, GPR_TIMESPAN));
-    ctx_.set_deadline(deadline);
-  }
-
   call_ = stub_->PrepareCall(&ctx_, method, &cq_);
   call_->StartCall(tag(1));
   void* got_tag;

test/cpp/util/cli_call.h

@@ -19,16 +19,14 @@
 #ifndef GRPC_TEST_CPP_UTIL_CLI_CALL_H
 #define GRPC_TEST_CPP_UTIL_CLI_CALL_H
 
+#include <map>
+
 #include <grpcpp/channel.h>
 #include <grpcpp/completion_queue.h>
 #include <grpcpp/generic/generic_stub.h>
 #include <grpcpp/support/status.h>
 #include <grpcpp/support/string_ref.h>
 
-#include <map>
-
-#include "test/cpp/util/cli_flags.h"
-
 namespace grpc {
 
 class ClientContext;

test/cpp/util/cli_flags.cc

@@ -1,53 +0,0 @@
-/*
- *
- * Copyright 2016 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-#include "test/cpp/util/cli_flags.h"
-
-namespace grpc {
-namespace testing {
-
-// Define all flags used in gRPC cli.
-DEFINE_bool(l, false, "Use a long listing format");
-DEFINE_bool(remotedb, true, "Use server types to parse and format messages");
-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(protofiles, "", "Name of the proto file.");
-DEFINE_bool(binary_input, false, "Input in binary format");
-DEFINE_bool(binary_output, false, "Output in binary format");
-DEFINE_string(
-    default_service_config, "",
-    "Default service config to use on the channel, if non-empty. Note that "
-    "this will be ignored if the name resolver returns a service config.");
-DEFINE_bool(display_peer_address, false,
-            "Log the peer socket address of the connection that each RPC is "
-            "made on to stderr.");
-DEFINE_bool(json_input, false, "Input in json format");
-DEFINE_bool(json_output, false, "Output in json format");
-DEFINE_string(infile, "", "Input file (default is stdin)");
-DEFINE_bool(batch, false,
-            "Input contains multiple requests. Please do not use this to send "
-            "more than a few RPCs. gRPC CLI has very different performance "
-            "characteristics compared with normal RPC calls which make it "
-            "unsuitable for loadtesting or significant production traffic.");
-DEFINE_double(timeout, -1,
-              "Specify timeout in seconds, used to set the deadline for all "
-              "RPCs. The default value of -1 means no deadline has been set.");
-
-}  // namespace testing
-}  // namespace grpc

test/cpp/util/cli_flags.h

@@ -1,46 +0,0 @@
-/*
- *
- * Copyright 2016 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-#ifndef GRPC_TEST_CPP_UTIL_CLI_FLAGS_H
-#define GRPC_TEST_CPP_UTIL_CLI_FLAGS_H
-
-#include <gflags/gflags.h>
-
-namespace grpc {
-namespace testing {
-
-// Declare all flags used in gRPC cli.
-DECLARE_bool(l);
-DECLARE_bool(remotedb);
-DECLARE_string(metadata);
-DECLARE_string(proto_path);
-DECLARE_string(protofiles);
-DECLARE_bool(binary_input);
-DECLARE_bool(binary_output);
-DECLARE_string(default_service_config);
-DECLARE_bool(display_peer_address);
-DECLARE_bool(json_input);
-DECLARE_bool(json_output);
-DECLARE_string(infile);
-DECLARE_bool(batch);
-DECLARE_double(timeout);
-
-}  // namespace testing
-}  // namespace grpc
-
-#endif  // GRPC_TEST_CPP_UTIL_CLI_FLAGS_H

test/cpp/util/grpc_tool.cc

@@ -18,14 +18,6 @@
 
 #include "test/cpp/util/grpc_tool.h"
 
-#include <grpc/grpc.h>
-#include <grpc/support/port_platform.h>
-#include <grpcpp/channel.h>
-#include <grpcpp/create_channel.h>
-#include <grpcpp/grpcpp.h>
-#include <grpcpp/security/credentials.h>
-#include <grpcpp/support/string_ref.h>
-
 #include <cstdio>
 #include <fstream>
 #include <iostream>
@@ -34,6 +26,15 @@
 #include <string>
 #include <thread>
 
+#include <gflags/gflags.h>
+#include <grpc/grpc.h>
+#include <grpc/support/port_platform.h>
+#include <grpcpp/channel.h>
+#include <grpcpp/create_channel.h>
+#include <grpcpp/grpcpp.h>
+#include <grpcpp/security/credentials.h>
+#include <grpcpp/support/string_ref.h>
+
 #include "test/cpp/util/cli_call.h"
 #include "test/cpp/util/proto_file_parser.h"
 #include "test/cpp/util/proto_reflection_descriptor_database.h"
@@ -48,6 +49,32 @@
 namespace grpc {
 namespace testing {
 
+DEFINE_bool(l, false, "Use a long listing format");
+DEFINE_bool(remotedb, true, "Use server types to parse and format messages");
+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(protofiles, "", "Name of the proto file.");
+DEFINE_bool(binary_input, false, "Input in binary format");
+DEFINE_bool(binary_output, false, "Output in binary format");
+DEFINE_string(
+    default_service_config, "",
+    "Default service config to use on the channel, if non-empty. Note "
+    "that this will be ignored if the name resolver returns a service "
+    "config.");
+DEFINE_bool(
+    display_peer_address, false,
+    "Log the peer socket address of the connection that each RPC is made "
+    "on to stderr.");
+DEFINE_bool(json_input, false, "Input in json format");
+DEFINE_bool(json_output, false, "Output in json format");
+DEFINE_string(infile, "", "Input file (default is stdin)");
+DEFINE_bool(batch, false,
+            "Input contains multiple requests. Please do not use this to send "
+            "more than a few RPCs. gRPC CLI has very different performance "
+            "characteristics compared with normal RPC calls which make it "
+            "unsuitable for loadtesting or significant production traffic.");
+
 namespace {
 
 class GrpcTool {
@@ -463,10 +490,7 @@ bool GrpcTool::CallMethod(int argc, const char** argv,
       "    --binary_input           ; Input in binary format\n"
       "    --binary_output          ; Output in binary format\n"
       "    --json_input             ; Input in json format\n"
-      "    --json_output            ; Output in json format\n"
+      "    --json_output            ; Output in json format\n" +
-      "    --timeout                ; Specify timeout (in seconds), used to "
-      "set the deadline for RPCs. The default value of -1 means no "
-      "deadline has been set.\n" +
       cred.GetCredentialUsage());
 
   std::stringstream output_ss;

test/cpp/util/grpc_tool.h

@@ -19,12 +19,11 @@
 #ifndef GRPC_TEST_CPP_UTIL_GRPC_TOOL_H
 #define GRPC_TEST_CPP_UTIL_GRPC_TOOL_H
 
-#include <grpcpp/support/config.h>
-
 #include <functional>
 
+#include <grpcpp/support/config.h>
+
 #include "test/cpp/util/cli_credentials.h"
-#include "test/cpp/util/cli_flags.h"
 
 namespace grpc {
 namespace testing {

test/cpp/util/grpc_tool_test.cc

@@ -30,7 +30,6 @@
 #include <grpcpp/server_context.h>
 #include <gtest/gtest.h>
 
-#include <chrono>
 #include <sstream>
 
 #include "src/core/lib/gpr/env.h"
@@ -55,8 +54,6 @@ using grpc::testing::EchoResponse;
   "Echo\n"                        \
   "Echo1\n"                       \
   "Echo2\n"                       \
-  "CheckDeadlineUpperBound\n"     \
-  "CheckDeadlineSet\n"            \
   "CheckClientInitialMetadata\n"  \
   "RequestStream\n"               \
   "ResponseStream\n"              \
@@ -73,10 +70,6 @@ using grpc::testing::EchoResponse;
   "{}\n"                                                                       \
   "  rpc Echo2(grpc.testing.EchoRequest) returns (grpc.testing.EchoResponse) " \
   "{}\n"                                                                       \
-  "  rpc CheckDeadlineUpperBound(grpc.testing.SimpleRequest) returns "         \
-  "(grpc.testing.StringValue) {}\n"                                            \
-  "  rpc CheckDeadlineSet(grpc.testing.SimpleRequest) returns "                \
-  "(grpc.testing.StringValue) {}\n"                                            \
   "  rpc CheckClientInitialMetadata(grpc.testing.SimpleRequest) returns "      \
   "(grpc.testing.SimpleResponse) {}\n"                                         \
   "  rpc RequestStream(stream grpc.testing.EchoRequest) returns "              \
@@ -116,6 +109,17 @@ DECLARE_string(ssl_target);
 namespace grpc {
 namespace testing {
 
+DECLARE_bool(binary_input);
+DECLARE_bool(binary_output);
+DECLARE_bool(json_input);
+DECLARE_bool(json_output);
+DECLARE_bool(l);
+DECLARE_bool(batch);
+DECLARE_string(metadata);
+DECLARE_string(protofiles);
+DECLARE_string(proto_path);
+DECLARE_string(default_service_config);
+
 namespace {
 
 const int kServerDefaultResponseStreamsToSend = 3;
@@ -173,29 +177,6 @@ class TestServiceImpl : public ::grpc::testing::EchoTestService::Service {
     return Status::OK;
   }
 
-  Status CheckDeadlineSet(ServerContext* context, const SimpleRequest* request,
-                          StringValue* response) override {
-    response->set_message(context->deadline() !=
-                                  std::chrono::system_clock::time_point::max()
-                              ? "true"
-                              : "false");
-    return Status::OK;
-  }
-
-  // Check if deadline - current time <= timeout
-  // If deadline set, timeout + current time should be an upper bound for it
-  Status CheckDeadlineUpperBound(ServerContext* context,
-                                 const SimpleRequest* request,
-                                 StringValue* response) override {
-    auto seconds = std::chrono::duration_cast<std::chrono::seconds>(
-        context->deadline() - std::chrono::system_clock::now());
-
-    // Returning string instead of bool to avoid using embedded messages in
-    // proto3
-    response->set_message(seconds.count() <= FLAGS_timeout ? "true" : "false");
-    return Status::OK;
-  }
-
   Status RequestStream(ServerContext* context,
                        ServerReader<EchoRequest>* reader,
                        EchoResponse* response) override {
@@ -881,93 +862,6 @@ TEST_F(GrpcToolTest, CallCommandRequestStreamWithBadRequestJsonInput) {
   ShutdownServer();
 }
 
-TEST_F(GrpcToolTest, CallCommandWithTimeoutDeadlineSet) {
-  // Test input "grpc_cli call CheckDeadlineSet --timeout=5000.25"
-  std::stringstream output_stream;
-
-  const std::string server_address = SetUpServer();
-  const char* argv[] = {"grpc_cli", "call", server_address.c_str(),
-                        "CheckDeadlineSet"};
-
-  // Set timeout to 5000.25 seconds
-  FLAGS_timeout = 5000.25;
-
-  EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
-                                   std::bind(PrintStream, &output_stream,
-                                             std::placeholders::_1)));
-
-  // Expected output: "message: "true"", deadline set
-  EXPECT_TRUE(nullptr !=
-              strstr(output_stream.str().c_str(), "message: \"true\""));
-  ShutdownServer();
-}
-
-TEST_F(GrpcToolTest, CallCommandWithTimeoutDeadlineUpperBound) {
-  // Test input "grpc_cli call CheckDeadlineUpperBound --timeout=900"
-  std::stringstream output_stream;
-
-  const std::string server_address = SetUpServer();
-  const char* argv[] = {"grpc_cli", "call", server_address.c_str(),
-                        "CheckDeadlineUpperBound"};
-
-  // Set timeout to 900 seconds
-  FLAGS_timeout = 900;
-
-  EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
-                                   std::bind(PrintStream, &output_stream,
-                                             std::placeholders::_1)));
-
-  // Expected output: "message: "true""
-  // deadline not greater than timeout + current time
-  EXPECT_TRUE(nullptr !=
-              strstr(output_stream.str().c_str(), "message: \"true\""));
-  ShutdownServer();
-}
-
-TEST_F(GrpcToolTest, CallCommandWithNegativeTimeoutValue) {
-  // Test input "grpc_cli call CheckDeadlineSet --timeout=-5"
-  std::stringstream output_stream;
-
-  const std::string server_address = SetUpServer();
-  const char* argv[] = {"grpc_cli", "call", server_address.c_str(),
-                        "CheckDeadlineSet"};
-
-  // Set timeout to -5 (deadline not set)
-  FLAGS_timeout = -5;
-
-  EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
-                                   std::bind(PrintStream, &output_stream,
-                                             std::placeholders::_1)));
-
-  // Expected output: "message: "false"", deadline not set
-  EXPECT_TRUE(nullptr !=
-              strstr(output_stream.str().c_str(), "message: \"false\""));
-
-  ShutdownServer();
-}
-
-TEST_F(GrpcToolTest, CallCommandWithDefaultTimeoutValue) {
-  // Test input "grpc_cli call CheckDeadlineSet --timeout=-1"
-  std::stringstream output_stream;
-
-  const std::string server_address = SetUpServer();
-  const char* argv[] = {"grpc_cli", "call", server_address.c_str(),
-                        "CheckDeadlineSet"};
-
-  // Set timeout to -1 (default value, deadline not set)
-  FLAGS_timeout = -1;
-
-  EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
-                                   std::bind(PrintStream, &output_stream,
-                                             std::placeholders::_1)));
-
-  // Expected output: "message: "false"", deadline not set
-  EXPECT_TRUE(nullptr !=
-              strstr(output_stream.str().c_str(), "message: \"false\""));
-
-  ShutdownServer();
-}
-
 TEST_F(GrpcToolTest, CallCommandResponseStream) {
   // Test input: grpc_cli call localhost:<port> ResponseStream "message:
   // 'Hello'"