Skip to content

Commit 47ddce3

Browse files
authored
fix: add workaround for driver manager venv path bug (#102)
This works around a bug we identified with the 1.8.0 driver managers when using virtual environments. They are supposed to locate drivers in `$VIRTUAL_ENV/etc/adbc/drivers` but they look in `$VIRTUAL_ENV/etc/adbc/`. There may be a patch release soon to fix this but, in the mean time, we can work around this by symlinking manifests from `$VIRTUAL_ENV/etc/adbc/drivers` to `$VIRTUAL_ENV/etc/adbc/`. The manifests use absolute paths so it's fine. This PR also adds an integration workflow which is how I tested my change. I don't think we want to run it on every single commit to the repo so we might discuss here when we might want to run something like this or if we want to run it in another repo.
1 parent a825723 commit 47ddce3

File tree

3 files changed

+90
-1
lines changed

3 files changed

+90
-1
lines changed

.github/workflows/integration.yml

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# Copyright 2025 Columnar Technologies Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: Integration
16+
17+
on:
18+
push:
19+
branches:
20+
- "main"
21+
pull_request:
22+
23+
concurrency:
24+
group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
25+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
26+
27+
permissions:
28+
contents: read
29+
30+
jobs:
31+
integration:
32+
name: Build & Integrate (${{ matrix.os }})
33+
runs-on: ${{ matrix.os }}
34+
strategy:
35+
fail-fast: false
36+
matrix:
37+
os: ["ubuntu-latest", "macos-latest", "windows-latest"]
38+
steps:
39+
- uses: actions/checkout@v4
40+
41+
- name: Install Go
42+
uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0
43+
with:
44+
go-version: 1.24.2
45+
cache: true
46+
cache-dependency-path: go.sum
47+
48+
- name: Install Python
49+
uses: actions/setup-python@v6
50+
with:
51+
python-version: "3.13"
52+
53+
- name: Build dbc
54+
run: |
55+
go build ./cmd/dbc
56+
shell: bash
57+
58+
- name: Run Integration Tests
59+
run: ./ci/scripts/run_integration.sh
60+
shell: bash

ci/scripts/run_integration.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/bin/sh
2+
3+
set -eux
4+
5+
python -m venv .venv
6+
7+
if [ -f ".venv/bin/activate" ]; then
8+
. ".venv/bin/activate"
9+
else
10+
. ".venv/Scripts/activate"
11+
fi
12+
13+
pip install adbc_driver_manager
14+
15+
./dbc install duckdb
16+
17+
python -c "from adbc_driver_manager import dbapi; dbapi.connect(driver=\"duckdb\");"

config/driver.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,24 @@ func createDriverManifest(location string, driver DriverInfo) error {
171171
}
172172
}
173173

174-
f, err := os.Create(filepath.Join(location, driver.ID+".toml"))
174+
manifest_path := filepath.Join(location, driver.ID+".toml")
175+
f, err := os.Create(manifest_path)
175176
if err != nil {
176177
return fmt.Errorf("error creating manifest %s: %w", driver.ID, err)
177178
}
178179
defer f.Close()
179180

181+
// Workaround for bug in Python driver manager packages. Version 1.8.0 of the
182+
// packages use the old ADBC_CONFIG_PATH path we originally had and not the
183+
// new ADBC_DRIVER_PATH (e.g., /etc/adbc instead of /etc/adbc/drivers).
184+
//
185+
// To work around this, we create a symlink on level up to the manifest we're
186+
// installing.
187+
//
188+
// TODO: Remove this when the driver managers are fixed (>=1.8.1).
189+
symlink := filepath.Join(location, "..", driver.ID+".toml")
190+
os.Symlink(manifest_path, symlink)
191+
180192
toEncode := tomlDriverInfo{
181193
ManifestVersion: currentManifestVersion,
182194
Name: driver.Name,

0 commit comments

Comments
 (0)