Add ADBC coverage matrix and live Azure checks#97
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens ADBC/Flight SQL support clarity and verification by adding an ADBC coverage matrix, updating the recommended Azure authentication flow, and expanding schema/protocol correctness and live backend checks (including JDBC).
Changes:
- Add ADBC coverage documentation and update README guidance/examples (including supported Azure auth flow and protocol support list).
- Implement Flight
GetSchemain the DuckDB Flight SQL server and tighten metadata schemas / SqlInfo constant alignment. - Expand automated checks: new unit/e2e assertions around
GetSchema, SqlInfo IDs, ADBC behaviors, and live Azure JDBC persistence testing.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lakehouse/server.py |
Implements get_schema, refines metadata schema nullability, and aligns SqlInfo constants/values to generated protobuf enums. |
tests/test_server.py |
Adds unit coverage for GetSchema and validates SqlInfo ID reporting using protobuf constants. |
tests/test_e2e.py |
Adds ADBC e2e coverage for execute_schema, execute_partitions, cancel behavior, GetObjects filters, and transaction/autocommit semantics. |
tests/test_e2e_tls.py |
Ensures autocommit is enabled when supported for TLS e2e connections. |
tests/test_e2e_ducklake.py |
Ensures autocommit is enabled when supported for DuckLake e2e connections. |
tests/test_live_azure_backend.py |
Expands live Azure smoke checks for ADBC features and adds a Maven-driven JDBC live test runner. |
tests/jdbc/pom.xml |
Passes live JDBC-related system properties through surefire to enable parameterized live runs. |
tests/jdbc/src/test/java/lakehouse/FlightSqlJdbcAzurePersistenceTest.java |
Adds a live JDBC test that verifies writes persist across connections against the deployed Container App. |
docs/adbc-coverage.md |
Introduces an ADBC coverage matrix documenting supported/partial/unsupported areas and caveats. |
README.md |
Links to the coverage matrix, updates Azure ADBC auth example to Basic-token bootstrap → Bearer, and clarifies supported protocol surface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise NotImplementedError(msg) | ||
|
|
There was a problem hiding this comment.
isinstance() cannot take a PEP 604 union (A | B) as the second argument; this will raise TypeError and break get_schema for imported/exported keys (and any command that reaches this branch). Use a tuple instead (e.g., isinstance(command, (fs.CommandGetImportedKeys, fs.CommandGetExportedKeys))).
| result = subprocess.run( | ||
| [ | ||
| "mvn", | ||
| "-q", | ||
| "test", | ||
| f"-Dflight.url={endpoint}", | ||
| "-Dflight.user=lakehouse", | ||
| f"-Dflight.password={password}", | ||
| "-Dlive.azure.jdbc.required=true", | ||
| f"-Dtest={test_class}", | ||
| ], |
There was a problem hiding this comment.
The JDBC live test passes -Dflight.password=... on the Maven command line. Command-line args are visible to other processes/users on the same machine (and can end up in CI logs), so this can leak the Key Vault secret. Consider passing the password via environment variable (and referencing ${env.FLIGHT_PASSWORD} in surefire), or another non-argv channel, and keep stdout/stderr redaction as defense-in-depth.
This pull request introduces several improvements and clarifications to ADBC (Arrow Database Connectivity) support and documentation, as well as enhancements to Flight SQL protocol handling and schema accuracy. The key changes include adding a detailed ADBC coverage matrix, updating the documentation and code examples for proper ADBC authentication flow, improving schema definitions for Flight SQL metadata, and aligning protocol constants with the generated protobuf values. These updates help clarify supported features, ensure protocol compliance, and improve the developer experience for both users and contributors.
ADBC Support Documentation and Guidance:
docs/adbc-coverage.mdto document compatibility, feature status, and known limitations for the Flight SQL server and Python ADBC driver.README.mdto point to the new ADBC coverage matrix and clarified which ADBC authentication paths are officially supported, including a warning not to use the direct Basic-auth path until it is promoted. [1] [2]Authentication and Usage Examples:
README.mdto use the supported PyArrow Basic-token handshake followed by Bearer token authentication, and updated the explanation of live backend test results for ADBC. [1] [2]Flight SQL Protocol and Schema Improvements:
get_schemamethod inlakehouse.server.FlightSqlServer, providing accurate schemas for all supported Flight SQL commands and ensuring side-effect-free behavior.lakehouse/server.pyto mark fields as non-nullable where appropriate, improving schema accuracy.Protocol Constant Alignment:
Build and Test Configuration:
tests/jdbc/pom.xmlto support live Azure JDBC testing with user and password parameters.