zeroshade commented on code in PR #3174:
URL: https://github.com/apache/arrow-adbc/pull/3174#discussion_r2216383547


##########
go/adbc/driver/bigquery/driver.go:
##########
@@ -77,6 +77,49 @@ const (
 
        AccessTokenEndpoint   = "https://accounts.google.com/o/oauth2/token";
        AccessTokenServerName = "google.com"
+
+       // UseAppDefaultCredentials instructs the driver to authenticate using
+       // Application Default Credentials (ADC).
+       OptionValueAuthTypeAppDefaultCredentials = "app_default_credentials"
+)
+
+// option that can be passed to the NewClient method.
+const (
+       // WithAccessToken instructs the driver to authenticate using the given
+       // access token.
+       WithAccessToken = "adbc.rpc.flightsql.auth.access_token"

Review Comment:
   should this follow the same pattern of `adbc.google.bigquery` instead of 
`adbc.rpc.flightsql`?



##########
go/adbc/driver/bigquery/bigquery_database.go:
##########
@@ -33,6 +34,15 @@ type databaseImpl struct {
        clientID     string
        clientSecret string
        refreshToken string
+
+       impersonateTargetPrincipal string
+       impersonateDelegates       []string
+       impersonateScopes          []string
+       impersonateLifetime        string // Changed from time.Duration to 
string

Review Comment:
   why the change to a string instead of `time.Duration`?



##########
go/adbc/driver/bigquery/driver.go:
##########
@@ -96,18 +139,32 @@ func init() {
 
 type driverImpl struct {
        driverbase.DriverImplBase
+       clientFactory      bigqueryClientFactory
+       tokenSourceFactory impersonatedTokenSourceFactory
 }
 
 // NewDriver creates a new BigQuery driver using the given Arrow allocator.
 func NewDriver(alloc memory.Allocator) adbc.Driver {
+       return NewDriverWithClientFactory(alloc, 
&defaultBigqueryClientFactory{})
+}
+
+// NewDriverWithClientFactory creates a new BigQuery driver using the given 
Arrow allocator and client factory.
+func NewDriverWithClientFactory(alloc memory.Allocator, clientFactory 
bigqueryClientFactory) adbc.Driver {
+       return NewDriverWithClientAndTokenSourceFactory(alloc, clientFactory, 
&defaultImpersonatedTokenSourceFactory{})
+}
+
+// NewDriverWithClientAndTokenSourceFactory creates a new BigQuery driver 
using the given Arrow allocator, client factory, and impersonated token source 
factory.
+func NewDriverWithClientAndTokenSourceFactory(alloc memory.Allocator, 
clientFactory bigqueryClientFactory, tokenSourceFactory 
impersonatedTokenSourceFactory) adbc.Driver {

Review Comment:
   should we add a nil check for the `clientFactory` to use 
`&defaultBigqueryClientFactory{}` if nil is passed in?



##########
go/adbc/driver/bigquery/bigquery_database.go:
##########
@@ -115,6 +131,8 @@ func (d *databaseImpl) SetOption(key string, value string) 
error {
                        d.authType = value
                case OptionValueAuthTypeUserAuthentication:
                        d.authType = value
+               case OptionValueAuthTypeAppDefaultCredentials:
+                       d.authType = value

Review Comment:
   Let's consolidate all of these instead of repeating `d.authType = value` 
over and over



-- 
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-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to