[GitHub] [arrow-cookbook] lidavidm commented on a change in pull request #153: [C++] gRPC settings and custom endpoint examples

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

lidavidm commented on a change in pull request #153:
URL: https://github.com/apache/arrow-cookbook/pull/153#discussion_r813944232



##########
File path: cpp/code/flight.cc
##########
@@ -43,9 +47,9 @@ class ParquetStorageService : public arrow::flight::FlightServerBase {
   explicit ParquetStorageService(std::shared_ptr<arrow::fs::FileSystem> root)
       : root_(std::move(root)) {}
 
-  arrow::Status ListFlights(const arrow::flight::ServerCallContext&,
-                            const arrow::flight::Criteria*,
-                            std::unique_ptr<arrow::flight::FlightListing>* listings) override {
+  arrow::Status ListFlights(
+      const arrow::flight::ServerCallContext&, const arrow::flight::Criteria*,
+      std::unique_ptr<arrow::flight::FlightListing>* listings) override {

Review comment:
       We should really set up clang-format properly 

##########
File path: cpp/code/flight.cc
##########
@@ -173,7 +177,41 @@ class ParquetStorageService : public arrow::flight::FlightServerBase {
   std::shared_ptr<arrow::fs::FileSystem> root_;
 };  // end ParquetStorageService
 
-arrow::Status TestPutGetDelete() {
+class HelloWorldServiceImpl : public HelloWorldService::Service {
+  grpc::Status SayHello([[maybe_unused]] grpc::ServerContext* ctx,
+                        const HelloRequest* request, HelloResponse* reply) override {
+    const std::string& name = request->name();
+    if (name.empty()) {
+      return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT, "Must provide a name!");
+    }
+    reply->set_reply("Hello, " + name);
+    return grpc::Status::OK;
+  }
+};  // end HelloWorldServiceImpl
+
+class HelloWorldClient {

Review comment:
       IMO, we can just inline the client code into our sample instead of writing out a wrapper (I don't feel like the wrapper adds much)

##########
File path: cpp/code/flight.cc
##########
@@ -173,7 +177,41 @@ class ParquetStorageService : public arrow::flight::FlightServerBase {
   std::shared_ptr<arrow::fs::FileSystem> root_;
 };  // end ParquetStorageService
 
-arrow::Status TestPutGetDelete() {
+class HelloWorldServiceImpl : public HelloWorldService::Service {
+  grpc::Status SayHello([[maybe_unused]] grpc::ServerContext* ctx,
+                        const HelloRequest* request, HelloResponse* reply) override {
+    const std::string& name = request->name();
+    if (name.empty()) {
+      return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT, "Must provide a name!");
+    }
+    reply->set_reply("Hello, " + name);
+    return grpc::Status::OK;
+  }
+};  // end HelloWorldServiceImpl
+
+class HelloWorldClient {
+ public:
+  HelloWorldClient(std::shared_ptr<grpc::Channel> channel)
+      : stub_(HelloWorldService::NewStub(channel)) {}
+
+  arrow::Result<std::string> HelloWorld(std::string name) {
+    grpc::ClientContext context;
+    HelloRequest request;
+    request.set_name(name);
+    HelloResponse response;
+    grpc::Status status = stub_->SayHello(&context, request, &response);
+    if (!status.ok()) {
+      return arrow::Status::IOError(status.error_message());
+    }
+    return response.reply();
+  }
+
+  protected:
+    std::unique_ptr<HelloWorldService::Stub> stub_;
+};  // end HelloWorldClient
+
+arrow::Status
+TestPutGetDelete() {

Review comment:
       Actually - it seems like a different formatter got used here? clang-format wouldn't do this

##########
File path: cpp/source/flight.rst
##########
@@ -83,3 +83,79 @@ Finally, we'll stop our server:
 
 .. recipe:: ../code/flight.cc ParquetStorageService::StopServer
    :dedent: 2
+
+
+Setting gRPC client options
+===========================
+
+Options for gRPC clients can be passed in using the ``generic_options`` field of
+:cpp:class:`arrow::flight::FlightClientOptions`. There is a list of available
+client options in the `gRPC API documentation <https://grpc.github.io/grpc/cpp/group__grpc__arg__keys.html>`_. 
+
+For example, you can change the maximum message length sent with:
+
+.. recipe:: ../code/flight.cc TestClientOptions::Connect
+   :dedent: 2
+
+
+Flight Service with other gRPC endpoints
+========================================
+
+If you are using the gRPC backend, you can add other gRPC endpoints to the 
+flight server. While Flight clients won't recognize these endpoints, general
+gRPC clients will be able to.
+
+.. note::
+   If statically linking Arrow Flight, Protobuf and gRPC must also be statically
+   linked, and the same goes for dynamic linking. Read more at
+   https://arrow.apache.org/docs/cpp/build_system.html#a-note-on-linking
+
+Creating the server
+-------------------
+
+To create a gRPC service, first define a service using protobuf.
+
+.. literalinclude:: ../code/protos/helloworld.proto
+   :language: protobuf
+   :linenos:
+   :start-at: syntax = "proto3";
+   :caption: Hello world protobuf specification
+
+Next, you'll need to compile that. If you are using CMake, consider the setup
+suggested by https://www.f-ax.de/dev/2020/11/08/grpc-plugin-cmake-support.html.

Review comment:
       I'm not sure it's worth linking to a random blog (no offense to the blog!)

##########
File path: cpp/code/protos/CMakeLists.txt
##########
@@ -0,0 +1,30 @@
+# Based on https://www.f-ax.de/dev/2020/11/08/grpc-plugin-cmake-support.html

Review comment:
       Hmm, is there a licensing concern here…?
   
   We could also base this on how Flight generates its protos. We can just include the Proto sources directly in our binary instead of as a library.

##########
File path: cpp/code/flight.cc
##########
@@ -173,7 +177,41 @@ class ParquetStorageService : public arrow::flight::FlightServerBase {
   std::shared_ptr<arrow::fs::FileSystem> root_;
 };  // end ParquetStorageService
 
-arrow::Status TestPutGetDelete() {
+class HelloWorldServiceImpl : public HelloWorldService::Service {
+  grpc::Status SayHello([[maybe_unused]] grpc::ServerContext* ctx,

Review comment:
       Instead of the attribute, we could also just declare it as `grpc::ServerContext*,...`




-- 
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: github-unsubscribe@arrow.apache.org

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