fix(destination-pgvector, destination-snowflake-cortex): adopt SQLAlchemy 2.x - #84401
fix(destination-pgvector, destination-snowflake-cortex): adopt SQLAlchemy 2.x#84401devin-ai-integration[bot] wants to merge 3 commits into
Conversation
…lchemy 2.x for pandas 2.2 pandas 2.2 requires SQLAlchemy >=2.0 for its optional-dependency check, so the stale sqlalchemy <2.0 pin made every write fail with 'Using URI string without sqlalchemy installed.' Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: bot_apk <apk@cognition.ai>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
Note Autopilot progressive rollouts are not enabled for the following modified connector(s):
This is a courtesy heads-up only — it does not block merge or fail any check. |
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
There was a problem hiding this comment.
Pull request overview
This PR fixes production sync failures in destination-pgvector and destination-snowflake-cortex caused by an incompatible pandas/SQLAlchemy pairing by updating both connectors to require SQLAlchemy 2.x and adding regression tests to prevent reintroducing the issue.
Changes:
- Update both connectors’
pyproject.tomlto requiresqlalchemy >=2.0.0,<3.0and relock to SQLAlchemy 2.0.52. - Add unit tests that exercise
pandas.DataFrame.to_sql(..., "<sqlalchemy-uri>", ...)with a URI string to guard against the original failure mode. - Bump connector versions in
metadata.yamland update destination docs changelogs.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/integrations/destinations/snowflake-cortex.md | Adds changelog entry for the dependency compatibility fix. |
| docs/integrations/destinations/pgvector.md | Adds changelog entry for the dependency compatibility fix. |
| airbyte-integrations/connectors/destination-snowflake-cortex/unit_tests/dependency_test.py | Adds regression test covering pandas.to_sql() with a URI string. |
| airbyte-integrations/connectors/destination-snowflake-cortex/pyproject.toml | Pins SQLAlchemy to 2.x-compatible range and bumps connector version. |
| airbyte-integrations/connectors/destination-snowflake-cortex/poetry.lock | Relocks dependencies to SQLAlchemy 2.0.52. |
| airbyte-integrations/connectors/destination-snowflake-cortex/metadata.yaml | Updates docker image tag to 0.2.31. |
| airbyte-integrations/connectors/destination-pgvector/unit_tests/dependency_test.py | Adds regression test covering pandas.to_sql() with a URI string. |
| airbyte-integrations/connectors/destination-pgvector/pyproject.toml | Pins SQLAlchemy to 2.x-compatible range and bumps connector version. |
| airbyte-integrations/connectors/destination-pgvector/poetry.lock | Relocks dependencies to SQLAlchemy 2.0.52. |
| airbyte-integrations/connectors/destination-pgvector/metadata.yaml | Updates docker image tag to 0.1.13. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def test_pandas_to_sql_accepts_sqlite_uri(): | ||
| pd.DataFrame({"value": [1]}).to_sql("table", "sqlite://", if_exists="append", index=False) |
There was a problem hiding this comment.
🚫 Not fixing. Same as the pgvector thread: the test only asserts that to_sql() with a URI string doesn't raise and never reads rows back, so in-memory persistence across reconnects isn't relied on. The bare sqlite:// form keeps the guard free of filesystem state, and the failure mode being guarded (pandas rejecting the URI-string form when its SQLAlchemy optional-dependency check fails) fires before any SQL is issued.
| def test_pandas_to_sql_accepts_sqlite_uri(): | ||
| pd.DataFrame({"value": [1]}).to_sql("table", "sqlite://", if_exists="append", index=False) |
There was a problem hiding this comment.
🚫 Not fixing. The reconnect concern doesn't apply here: this test only asserts that to_sql() with a URI string doesn't raise, it never reads the rows back, so in-memory table persistence isn't something it depends on. The bare sqlite:// form is also deliberate — it keeps the guard free of filesystem state, and pandas creates exactly one engine/connection for the call.
The failure mode being guarded is pandas' optional-dependency check rejecting the URI-string form, which fires before any SQL is issued, so the URL flavor doesn't affect what this catches.
|
|
↪️ Triggering Reason: Draft PR, CI checks passing, and no |
|
🔴 Fix Disproven — incompleteRequiring SQLAlchemy 2.x does clear the reported Before/after comparison (isolated, synthetic data only) Local pgvector/Postgres 16 container + PyAirbyte's
Why SQLAlchemy 2.0 rejects raw SQL strings passed to
Suggested next step: wrap those statements in Pre-flight checks
Detailed results including the affected connections: see |
Airbyte Support Bot (airbyte-support-bot)
added
the
hyd-prove
|
|
|
🙋 Escalated per Hands-Free AI Triage Project — this PR is remediating a live production failure. A post-merge health check on #84364 shows That makes this draft the fix for an active breakage rather than routine hardening. |
Co-Authored-By: bot_apk <apk@cognition.ai>
devin-ai-integration
Bot
changed the title
|
☑️ Resolved in dbfb10d — thanks, the prove-fix disproof was correct: the dependency change alone left the SQLAlchemy 1.4-only call sites broken under 2.0. What changed:
Test coverage now includes the dedup path, per your note that the Live re-validation against a local Versions were intentionally not bumped again — 0.1.13 / 0.2.31 were never published, so the existing changelog rows were extended instead. Ready for another prove-fix pass whenever convenient. Link to Devin session: https://app.devin.ai/sessions/fba3851ccb9e45e18322f5db3cada037 |
|
↪️ Triggering Reason: the earlier prove-fix on this branch returned 🔴 Fix Disproven (SQLAlchemy 2.0 broke a second spot in the same write path, so dedup-mode streams still failed), and the author has since pushed |
|
🟢 Fix ProvenThe reported failure reproduces on the currently published Next steps: review and merge, then run Detailed results, including the full evidence log: airbytehq/oncall#13306 (internal). Devin session Connector, versions and pre-releases tested
Evidence plan
Cases attempted
Pre-flight checks
Evidence logBaseline reproduction — published
This matches the reported failure, including the frame it is raised from. Fix verification —
The dedup path is the one that failed both in production and on the previous revision of this PR, so this is the discriminating result. Unit tests on the PR branch
The Cortex merge test runs the real Cortex pre-release image —
Caveat A Snowflake-backed write was never executed, so Snowflake-dialect-specific behavior of the wrapped statements (the |
|
|
|
↪️ Triggering Reason: the re-run |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review ReportReview ActionAPPROVE — all enforced gates pass. One non-blocking WARNING on Breaking Dependencies (major SQLAlchemy upgrade). Risk Level3 / 5 (medium) — typical functional connector change. The PR upgrades SQLAlchemy from Gates Needing Attention
All other 11 gates: PASS. PR Details
Gate Evaluation Details1. PR Hygiene — PASS 2. Code Hygiene — PASS 3. Test Coverage — PASS 4. Code Security — PASS 5. Per-Record Performance — PASS 6. Breaking Dependencies — WARNING 7. Backwards Compatibility — PASS 8. Forwards Compatibility — PASS 9. Behavioral Changes — PASS 10. Out-of-Scope Changes — PASS 11. CI Checks — PASS 12. Live / E2E Tests — PASS Evidence Consulted
|
|
↪️ Triggering Reason: the |
Auto-merge evaluation: FAILEligibility (all must pass)
|
|
💬 Readiness outcome for the record, per Hands-Free AI Triage Project: |
|
🙋 Escalated to #human-in-the-loop per Hands-Free AI Triage Project triage next step. Reason: CI is green, |
What
Resolves https://github.com/airbytehq/oncall/issues/13306:
Every sync on
destination-pgvector0.1.12 fails at the write stage with:raised from
pandas/io/sql.py::pandasSQL_builder.Both connectors' lockfiles resolve
pandas==2.2.3, while theirpyproject.tomlstill declared a stalesqlalchemy = "<2.0"pin, so the images shippedsqlalchemy==1.4.54. pandas 2.2.x requires SQLAlchemy>=2.0.0in its optional-dependency check (2.1.4 required>=1.4.36), soimport_optional_dependency("sqlalchemy", errors="ignore")returnsNoneand pandas rejects the URI-string form ofDataFrame.to_sql()— which is exactly the form used incommon/sql/sql_processor.py::_write_files_to_new_table.destination-snowflake-cortex0.2.30 has the identical dependency shape and the same call site, so it is fixed here too.Moving to SQLAlchemy 2.x also requires fixing the connectors' remaining SQLAlchemy 1.4-only call sites: 2.0 no longer accepts raw SQL strings in
Connection.execute(). A/ai-prove-fixrun on the first revision of this PR caught this — append-only writes passed, but dedup writes failed withsqlalchemy.exc.ObjectNotExecutableError: Not an executable object: '\nDELETE FROM ...'.How
Two parts:
Dependencies: replaced the stale pin with
sqlalchemy = ">=2.0.0,<3.0"in both connectors and relocked. SQLAlchemy 2.x is the version this code targets — the vendoredcommon/sql/sql_processor.pyis a copy of PyAirbyte's shared SQL processor, and upstream PyAirbyte pinssqlalchemy==2.0.43.snowflake-sqlalchemy1.11.0 requires onlysqlalchemy>=1.4.19and works under 2.0.52;pgvector0.3.2 supports 2.x. The relock is contained: the only package version change in either lockfile issqlalchemy 1.4.54 → 2.0.52.SQLAlchemy 2.0 call sites, in the connector-specific overrides that bypass
_execute_sql(which already wraps strings):pgvector_processor.py::_emulated_merge_temp_table_to_final_tableandcortex_processor.py::_emulated_merge_temp_table_to_final_table— the DELETE/INSERT statements are now wrapped insqlalchemy.text(), still inside oneget_sql_connection()transaction.cortex_processor.py::_init_connection_settings— theALTER SESSION SET ...statement likewise. This runs on every connection, so unfixed it would breakcheckand every Cortex sync regardless of sync mode.common/sql/sql_processor.pyin both connectors:select([temp_table])→select(temp_table)(the 1.4 list form is rejected by 2.0; matches upstream PyAirbyte), and dropped the 1.4-onlyexecution_options(autocommit=True)block —get_sql_connection()usesengine.begin(), so statements are committed regardless.Review guide
*/pyproject.toml— the constraint change*/poetry.lock— only sqlalchemy changes versionpgvector_processor.py,cortex_processor.py— thetext()wraps*/common/sql/sql_processor.py— the two 1.4-only patterns*/unit_tests/dependency_test.py— regression guardsTest Coverage
unit_tests/dependency_test.pyin both connectors now covers both failure modes:DataFrame.to_sql(..., "sqlite://", ...)— the pandas/SQLAlchemy pair. In a scratch venv withpandas==2.2.3+sqlalchemy==1.4.54it fails with the exact productionImportError; passes in both fixed environments._emulated_merge_temp_table_to_final_tableexecuted for real against temporary SQLite — reverting thetext()wraps makes both connectors' tests fail withObjectNotExecutableError._init_connection_settings— asserts the statement handed toConnection.executeis aTextClause, not astr(ALTER SESSIONhas no SQLite equivalent to execute).Local runs (unit test suites are currently disabled in both connectors'
metadata.yaml, so these are the signal):poetry run pytest unit_tests/ -x— destination-pgvector: 7 passed; destination-snowflake-cortex: 8 passedLive write-path validation for pgvector against a local
pgvector/pgvector:pg16container (CDK fake embedder):checkreturnedSUCCEEDEDNo Snowflake credentials were available, so
destination-snowflake-cortexwas validated by unit tests plus dependency/dialect resolution under SQLAlchemy 2.0.52, not a live write. A Cortex prerelease or regression run before release would be worthwhile.User Impact
Syncs on both destinations start working again, in both append and dedup modes. No spec, schema, stream, state, or behavior changes, so this is a patch bump for each connector (0.1.13 and 0.2.31) and is not a breaking change.
Can this PR be safely reverted and rolled back?
Requested by Devin Bot via the
/ai-fixworkflow on https://github.com/airbytehq/oncall/issues/13306.Link to Devin session: https://app.devin.ai/sessions/fba3851ccb9e45e18322f5db3cada037