You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/03/02 14:55:25 UTC

[GitHub] [arrow-rs] alamb commented on a diff in pull request #3788: refactor: assorted `FlightSqlServiceClient` improvements

alamb commented on code in PR #3788:
URL: https://github.com/apache/arrow-rs/pull/3788#discussion_r1123226359


##########
arrow-flight/src/sql/client.rs:
##########
@@ -83,13 +75,23 @@ impl FlightSqlServiceClient {
 
     /// Creates a new HTTPs FlightSql Client that connects via TCP to a server
     #[cfg(feature = "tls")]
-    pub async fn new_with_endpoint(
-        client_ident: Identity,
-        server_ca: Certificate,
-        domain: &str,
+    pub async fn new_with_tls_endpoint(

Review Comment:
   what do you think about removing all tls configuration out of the actual client and instead add a doc example that shows how to configure the client for TLS?
   
   I feel like simply having `new(channel: Channel)` would be the simplest API and all the rest of the code in this function could be turned into doc comments. This would likely be simpler to use, have less code, and be easier to configure with arbitrary configurations (like keepalives, etc)



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