lidavidm commented on code in PR #3174:
URL: https://github.com/apache/arrow-adbc/pull/3174#discussion_r2229944828
##########
go/adbc/driver/bigquery/driver.go:
##########
@@ -96,18 +135,35 @@ 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 {
+ if clientFactory == nil {
+ clientFactory = &defaultBigqueryClientFactory{}
+ }
+ 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:
Ditto here, there are private types in the signature, the function should be
private
##########
go/adbc/driver/bigquery/connection.go:
##########
@@ -248,6 +258,35 @@ type bigQueryTokenResponse struct {
TokenType string `json:"token_type"`
}
+// bigqueryClientFactory is an interface for creating BigQuery clients.
+// This is used for mocking in tests.
+type bigqueryClientFactory interface {
+ NewClient(ctx context.Context, projectID string, opts
...option.ClientOption) (*bigquery.Client, error)
+ EnableStorageReadClient(ctx context.Context, client *bigquery.Client,
opts ...option.ClientOption) error
Review Comment:
Given we always enable the storage client, we should just bundle these
functions together
##########
go/adbc/driver/bigquery/driver.go:
##########
@@ -77,6 +77,45 @@ 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 (
+ // WithAppDefaultCredentials instructs the driver to authenticate using
+ // Application Default Credentials (ADC).
+ WithAppDefaultCredentials =
"adbc.google.bigquery.use_application_default_credentials"
+
+ // WithJSONCredentials instructs the driver to authenticate using the
+ // given JSON credentials. The value should be a byte array representing
+ // the JSON credentials.
+ WithJSONCredentials = "adbc.google.bigquery.json_credentials"
+
+ // WithOAuthClientIDs instructs the driver to authenticate using the
given
+ // OAuth client ID and client secret. The value should be a string array
+ // of length 2, where the first element is the client ID and the second
+ // is the client secret.
+ WithOAuthClientIDs = "adbc.google.bigquery.oauth_client_ids"
Review Comment:
These options are still inconsistent with the rest? (Also why
`adbc.google.bigquery` vs `adbc.bigquery`?)
##########
go/adbc/driver/bigquery/connection.go:
##########
@@ -248,6 +258,35 @@ type bigQueryTokenResponse struct {
TokenType string `json:"token_type"`
}
+// bigqueryClientFactory is an interface for creating BigQuery clients.
+// This is used for mocking in tests.
Review Comment:
I don't see this used in tests, though?
##########
go/adbc/driver/bigquery/connection.go:
##########
@@ -248,6 +258,35 @@ type bigQueryTokenResponse struct {
TokenType string `json:"token_type"`
}
+// bigqueryClientFactory is an interface for creating BigQuery clients.
+// This is used for mocking in tests.
+type bigqueryClientFactory interface {
+ NewClient(ctx context.Context, projectID string, opts
...option.ClientOption) (*bigquery.Client, error)
+ EnableStorageReadClient(ctx context.Context, client *bigquery.Client,
opts ...option.ClientOption) error
Review Comment:
In any case, `EnableStorageReadClient` is a method of `bigquery.Client` so
it makes no sense in this interface
##########
go/adbc/driver/bigquery/driver.go:
##########
@@ -96,18 +135,35 @@ 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 {
Review Comment:
Since `bigqueryClientFactory` is a private type, this function should also
be private
--
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
