lidavidm commented on code in PR #4009:
URL: https://github.com/apache/arrow-adbc/pull/4009#discussion_r2850182689
##########
go/adbc/adbc.go:
##########
@@ -818,3 +907,35 @@ type GetSetOptions interface {
GetOptionInt(key string) (int64, error)
GetOptionDouble(key string) (float64, error)
}
+
+// PostInitOptionsWithContext is a PostInitOptions that supports
context.Context.
+type PostInitOptionsWithContext interface {
Review Comment:
I guess that may make the wrapping helpers too complicated
##########
go/adbc/adbc.go:
##########
@@ -33,6 +33,18 @@
// safely from multiple goroutines, but not necessarily concurrent
// access. Specific implementations may allow concurrent access.
//
+// # Context Support
+//
+// Context-aware interfaces are available:
+// DatabaseContext, ConnectionContext, and StatementContext. These interfaces
Review Comment:
nit: these should be renamed
##########
go/adbc/adbc.go:
##########
@@ -818,3 +907,35 @@ type GetSetOptions interface {
GetOptionInt(key string) (int64, error)
GetOptionDouble(key string) (float64, error)
}
+
+// PostInitOptionsWithContext is a PostInitOptions that supports
context.Context.
+type PostInitOptionsWithContext interface {
Review Comment:
I wonder if we should just fold this into GetSetOptionsWithContext, IMO we
can just expect everything to support this as a matter of course? And maybe
even require DatabaseWithContext etc. to embed GetSetOptionsWithContext?
##########
go/adbc/context_adapters.go:
##########
@@ -0,0 +1,193 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package adbc
+
+import (
+ "context"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/array"
+)
+
+// databaseContextAdapter wraps a Database to implement DatabaseContext.
+type databaseContextAdapter struct {
+ db Database
+}
+
+// AsDatabaseContext wraps a Database to implement DatabaseContext.
+// This adapter allows using a non-context Database implementation with
+// context-aware code. The context parameter is used for methods that
+// already accept context (like Open), but is effectively ignored for
+// methods that don't (like Close, SetOptions) since the underlying
+// implementation cannot respond to cancellation or deadlines.
+func AsDatabaseContext(db Database) DatabaseWithContext {
Review Comment:
I wonder if we should call this `AsDatabaseWithContext`?
Too verbose?
##########
go/adbc/adbc.go:
##########
@@ -341,6 +353,19 @@ type Driver interface {
NewDatabase(opts map[string]string) (Database, error)
}
+// DriverWithContext is an extension interface to allow the creation of a
database
+// by providing an existing [context.Context] to initialize OpenTelemetry
tracing.
+// It is similar to [database/sql.Driver] taking a map of keys and values as
options
+// to initialize a [Connection] to the database. Any common connection
+// state can live in the Driver itself, for example an in-memory database
+// can place ownership of the actual database in this driver.
+//
+// Any connection specific options should be set using SetOptions before
+// calling Open.
+type DriverWithContext interface {
+ NewDatabaseWithContext(ctx context.Context, opts map[string]string)
(Database, error)
Review Comment:
Given we aren't suffixing the other methods, maybe it's ok to break this
(experimental) API and call it just `NewDatabase` now?
--
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]
