[GitHub] [arrow] srebhan commented on pull request #34331: GH-34332: [Go][FlightRPC] Add driver for `database/sql` framework

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

srebhan commented on PR #34331:
URL: https://github.com/apache/arrow/pull/34331#issuecomment-1453362074

   @zeroshade thanks again for your thorough review! I've fixed most of your comments (at least IMO ;-)). As I'm not used to your usual habits I left the comments unresolved. Please let me know if I should resolve them by myself or let you go through those.
   
   There are a few point that are open
   1. Unification of backends: I think I should not deal with backend-specific settings at that level. So my idea is to unify those and let the user do the specifics on the `DSN` level. I hope to finish this next Friday...
   2. TLS implementation/customization: My plan is to implement a mechanism similar to the [MySQL driver](https://github.com/go-sql-driver/mysql) to have a driver-global TLS-config registry. I thought about doing this in a separate PR once this one is merged. What do you think?
   3. Moving the driver to another location: You suggested to move this to a separate directory. I'm fine with this, but I'm concerned about testing. The driver, by nature, is closely bound to the API and behavior of the FlightSQL client, therefore it's tests should run whenever something in the client changes. In a separate directory chances are high that the tests are forgotten... Anyway, after unification (see item 1) only `driver.go` and `driver_test.go` will be left. Do you want a separate dir nonetheless?
   
   Thanks again for your support and comments! Much appreciated.


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