[GitHub] [arrow-rs] tustvold commented on a diff in pull request #4144: Better flight SQL example codes

["tustvold (via GitHub)" <gi...@apache.org>]

tustvold commented on code in PR #4144:
URL: https://github.com/apache/arrow-rs/pull/4144#discussion_r1178809492


##########
arrow-flight/examples/flight_sql_server.rs:
##########
@@ -690,16 +648,89 @@ mod tests {
         }
     }
 
+    async fn test_tcp_client<F, C>(f: F)
+    where
+        F: FnOnce(FlightSqlServiceClient<Channel>) -> C,
+        C: Future<Output = ()>,
+    {
+        let addr = "127.0.0.1:50051";
+
+        // We would just listen on TCP, but it seems impossible to know when tonic is ready to serve

Review Comment:
   ```suggestion
   ```
   
   I believe the sleep below is to handle this



##########
arrow-flight/Cargo.toml:
##########
@@ -34,17 +34,37 @@ arrow-cast = { workspace = true }
 arrow-ipc = { workspace = true }
 arrow-schema = { workspace = true }
 base64 = { version = "0.21", default-features = false, features = ["std"] }
-tonic = { version = "0.9", default-features = false, features = ["transport", "codegen", "prost"] }
+tonic = { version = "0.9", default-features = false, features = [

Review Comment:
   I personally find this formatting harder to read...



##########
arrow-flight/examples/flight_sql_server.rs:
##########
@@ -690,16 +648,89 @@ mod tests {
         }
     }
 
+    async fn test_tcp_client<F, C>(f: F)
+    where
+        F: FnOnce(FlightSqlServiceClient<Channel>) -> C,
+        C: Future<Output = ()>,
+    {
+        let addr = "127.0.0.1:50051";

Review Comment:
   https://github.com/apache/arrow-rs/pull/4145 shows how this could be adapted to not need to hard-code a port



##########
arrow-flight/examples/flight_sql_server.rs:
##########
@@ -690,16 +648,89 @@ mod tests {
         }
     }
 
+    async fn test_tcp_client<F, C>(f: F)
+    where
+        F: FnOnce(FlightSqlServiceClient<Channel>) -> C,
+        C: Future<Output = ()>,
+    {
+        let addr = "127.0.0.1:50051";
+
+        // We would just listen on TCP, but it seems impossible to know when tonic is ready to serve
+        let service = FlightSqlServiceImpl {};
+        let serve_future = Server::builder()
+            .add_service(FlightServiceServer::new(service))
+            .serve(addr.parse().unwrap());
+
+        let request_future = async {
+            sleep(Duration::from_millis(2000)).await;

Review Comment:
   ```suggestion
               // wait for server to start up
               sleep(Duration::from_millis(2000)).await;
   ```



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