xborder commented on code in PR #4488:
URL: https://github.com/apache/arrow-adbc/pull/4488#discussion_r3551057032


##########
docs/source/driver/flight_sql.rst:
##########
@@ -81,6 +81,45 @@ the :c:struct:`AdbcDatabase`.
 
       .. recipe:: ../../../go/adbc/driver/flightsql/example_usage_test.go
 
+URI Format
+----------
+
+The "uri" option accepts URIs using the ``flightsql://`` scheme.  The
+transport is selected with the ``transport`` query parameter, which is
+matched case-insensitively:
+
+``flightsql://<host>:<port>``
+    Connect over gRPC with TLS (secure).  This is the default when no
+    ``transport`` query parameter is given.
+
+``flightsql://<host>:<port>?transport=tls``
+    Same as above; an explicit way to request TLS (secure).
+
+``flightsql://<host>:<port>?transport=tcp``
+    Connect over gRPC without TLS (plaintext).
+
+``flightsql:///<path/to/socket>?transport=unix``
+    Connect over gRPC via a Unix domain socket at the given path.
+
+An unrecognized ``transport`` value (anything other than ``tls``,
+``tcp``, or ``unix``) is rejected with an error rather than silently
+falling back to a default.  Similarly, supplying a host with
+``transport=unix``, or a socket path with ``transport=tcp`` or the
+default TLS transport, is rejected as an invalid combination.
+
+.. note:: For backwards compatibility, the driver also still accepts

Review Comment:
   If we're still allowing legacy schemes for backwards compatibility, what is 
the benefit of having `transport=tcp|tls|unix`?



##########
docs/source/driver/flight_sql.rst:
##########
@@ -81,6 +81,45 @@ the :c:struct:`AdbcDatabase`.
 
       .. recipe:: ../../../go/adbc/driver/flightsql/example_usage_test.go
 
+URI Format
+----------
+
+The "uri" option accepts URIs using the ``flightsql://`` scheme.  The
+transport is selected with the ``transport`` query parameter, which is
+matched case-insensitively:
+
+``flightsql://<host>:<port>``
+    Connect over gRPC with TLS (secure).  This is the default when no
+    ``transport`` query parameter is given.
+
+``flightsql://<host>:<port>?transport=tls``

Review Comment:
   Would it make sense to use something similar to `useEncryption=true/false` 
like in flightsql jdbc driver?  Is there something similar on another ADBC 
driver?
   
   I'd suggest `flightsql://<host>:<port>` to default to grpc+tls and maybe 
allow `useEncryption/ssl=true|false` like other APIs do. Is there a similar 
example in ADBC that we can replicate?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to