You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/07 21:20:04 UTC

[GitHub] [arrow] coryan commented on a diff in pull request #13520: ARROW-14889: [C++] GCS tests hang if testbench not installed

coryan commented on code in PR #13520:
URL: https://github.com/apache/arrow/pull/13520#discussion_r916289559


##########
cpp/src/arrow/filesystem/gcsfs_test.cc:
##########
@@ -105,9 +106,21 @@ class GcsTestbench : public ::testing::Environment {
         error += " (exe not found)";
         continue;
       }
-
-      server_process_ = bp::child(exe_path, "-m", "testbench", "--port", port_, group_);
-      if (server_process_.valid() && server_process_.running()) break;
+      // TODO: should I be worried about lifetime of this variable?
+      bp::ipstream output;
+
+      server_process_ = bp::child(exe_path, "-m", "testbench", "--port", port_, group_,
+                                  bp::std_err > output);
+      // Wait for message: "* Restarting with"
+      auto testbench_is_running = [&output](bp::child& process) {
+        std::string line;
+        while (process.valid() && process.running() && std::getline(output, line)) {
+          if (line.find("* Restarting with") != std::string::npos) return true;
+        }
+        return false;
+      };
+
+      if (testbench_is_running(server_process_)) break;

Review Comment:
   It depends on the value of "easy".  I have not tested this, but allegedly you can use asynchronous I/O with `Boost.Process`:
   
   https://www.boost.org/doc/libs/1_79_0/doc/html/process/reference.html#header.boost.process.io_hpp
   
   The example where they use a future could be changed with a `wait_for()`:
   
   ```cc
   boost::asio::io_context ios;
   std::future<std::string> output;
   system("ls", std_out > output, ios);
   
   auto status = fut.wait_for(std::chrono::seconds(10));
   if (status == std::future_status::timeout) { /* uh oh */ }
   ```
   
   I expect you would need a thread or something to run with the `boost::asio::io_context`, something like:
   
   ```cc
     boost::asio::io_context ios;
     std::thead svc{[&] { ios.run_for(std::chrono::seconds(..)); }};
   ...
     // insert the code from above
     svc.join();
   ....
   



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