-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(python): ADBC driver compatibility at write_database()
#25889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
write_database()
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25889 +/- ##
==========================================
- Coverage 80.62% 80.49% -0.14%
==========================================
Files 1765 1768 +3
Lines 243158 243107 -51
Branches 3044 3036 -8
==========================================
- Hits 196055 195686 -369
- Misses 46321 46640 +319
+ Partials 782 781 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! However, a hardcoded list isn't really a robust solution, and adds ongoing maintenance (if a new driver comes out we'd have to known about it somehow, update the list, and then push out a whole new release to pick up that support).
Is there no way to use the ADBC driver manager to confirm/discover drivers installed via dbc instead, if the module isn't found/available via a standard import? Then we would have automatic discovery as new drivers are made available.
…l adbc_get_info version
ADBC seems to hide a lot of stuff behind driver manager's dbapi and packed into c extension. or Unless there are additional ways to initialize a connection without using an actual driver package (how), this should suffice. The additional try-except behavior is only for compatibility cases where if a python package driver is available, its python version does not match its internal adbc_get_info version. @henryharbeck Would you mind give some clarification on the change you made, where you chose to use the python package version in adbc driver instead of its internal |
|
I didn't find Here's what it looks like for all the drivers I can get working at the momentimport warnings
from adbc_driver_bigquery import dbapi as bigquery_dbapi
from adbc_driver_duckdb import dbapi as duckdb_dbapi
from adbc_driver_manager import dbapi as manager_dbapi
from adbc_driver_postgresql import dbapi as postgresql_dbapi
from adbc_driver_sqlite import dbapi as sqlite_dbapi
# BigQuery complains
warnings.filterwarnings(
"ignore",
message="Cannot disable autocommit; conn will not be DB-API 2.0 compliant",
)
def main():
postgresql_uri = "postgresql://postgres:postgres@localhost:5432"
gcp_kwargs = {
"adbc.bigquery.sql.project_id": "...",
"adbc.bigquery.sql.dataset_id": "adbc_test",
}
with (
# installed with `uv add duckb`
duckdb_dbapi.connect() as duck_conn,
# installed with `uv add adbc-driver-(bigquery|postgresql|sqlite)`
bigquery_dbapi.connect(db_kwargs=gcp_kwargs) as bq_conn,
postgresql_dbapi.connect(postgresql_uri) as pg_conn,
sqlite_dbapi.connect() as sqlite_conn,
# installed with `dbc add (duckdb|bigquery|postgresql|sqlite)`
manager_dbapi.connect("duckdb") as manager_duck_conn,
manager_dbapi.connect("bigquery", db_kwargs=gcp_kwargs) as manager_bq_conn,
manager_dbapi.connect("postgresql", uri=postgresql_uri) as manager_pg_conn,
manager_dbapi.connect("sqlite") as manager_sqlite_conn,
):
connections = {
"duck_conn": duck_conn,
"bq_conn": bq_conn,
"pg_conn": pg_conn,
"sqlite_conn": sqlite_conn,
"manager_duck_conn": manager_duck_conn,
"manager_bq_conn": manager_bq_conn,
"manager_pg_conn": manager_pg_conn,
"manager_sqlite_conn": manager_sqlite_conn,
}
for conn_name, conn in connections.items():
assert isinstance(conn, manager_dbapi.Connection)
version = conn.adbc_get_info().get("driver_version", "<key not found>")
print(f"{conn_name}: {version = }")
if __name__ == "__main__":
main()outputs As an side, all the driver version checks are workarounds for versions that are now quite old. @alexander-beedie , what are you thoughts on setting a minimum adbc-driver- pypi version of 0.11 (released April 1, 2024)? This allows the removal of the driver pypi version checks and deals with the fact that when installed with Caveat being I'm not exactly sure how this can be enforced (outside of SQLite with the |
Issue
ADBC drivers issued by Columnar through
dbc addcommand are installed at .venv/etc/adbc/drivers. These drivers does not have a python package available, and are accessed through:While this is compatible with adbc_driver_manager, write_database function will raise an error in
_import_optional_adbc_driver()when trying to import an nonexistent python packageadbc_driver_DRIVER_NAME.Fix
This fix provides a naive fix by adding a range of existing pypi drivers into code prior to checking. If name is not in the list, behavior of assuming driver version equals manager version are assumed.
Checks
Performed write check on an dbc installed mysql driver.