Skip to content

Upgrade SQLAlchemy from 1.4 to 2.0#272

Open
kaapstorm wants to merge 16 commits into
masterfrom
nh/sqla_20
Open

Upgrade SQLAlchemy from 1.4 to 2.0#272
kaapstorm wants to merge 16 commits into
masterfrom
nh/sqla_20

Conversation

@kaapstorm
Copy link
Copy Markdown
Contributor

This branch upgrades SQLAlchemy from 1.4 to 2.0. This is required by the changes in PR #269 for the transaction management introduced in SQLAlchemy 2.0.

Each commit is small, or covers a small change across many files. Automated changes by ruff are in their own commits.

Comment thread commcare_export/checkpoint.py Outdated
jingcheng16
jingcheng16 previously approved these changes May 4, 2026
Copy link
Copy Markdown
Contributor

@jingcheng16 jingcheng16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know nothing about SQLAlchemy, so I just reviewed and make sure the code changes follow the commit messages. Can approve again when conflicts are resolved.

Base automatically changed from nh/write_table to master May 13, 2026 18:15
@kaapstorm kaapstorm dismissed jingcheng16’s stale review May 13, 2026 18:15

The base branch was changed.

kaapstorm and others added 9 commits May 13, 2026 14:22
Remove MetaData(bind=...), autoload=True, declarative_base() import,
and branched-connection pattern in env.py.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
IntegrityError handling no longer aborts the outer transaction.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Raw SQL strings must be wrapped in text() in SQLAlchemy 2.0. Also
convert positional %s parameters to named :name bindings, and add
conn.commit() calls after DDL operations now that autocommit has
been removed.
engine.execute() was removed in SQLAlchemy 2.0. Callers must obtain
a connection via engine.connect() and execute statements on it.
In SQLAlchemy 2.0 rows are tuple-like by default. Call .mappings() on
the Result to iterate dict-like RowMapping objects.
Accessing the raw DB-API connection via Connection.connection.connection
is deprecated in SQLAlchemy 2.0. Use Connection.connection.dbapi_connection
instead.
@kaapstorm kaapstorm force-pushed the nh/sqla_20 branch 2 times, most recently from 93a3333 to 8c0844e Compare May 13, 2026 20:21
kaapstorm and others added 7 commits May 13, 2026 16:35
Replace raw SQL COMMIT with SQLAlchemy 2.0 transaction API:
- __enter__/__exit__ manage begin/commit/rollback
- _flush() commits current transaction and starts a new one
- _flush_batch() rolls back failed transaction before retry

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SQLAlchemy 2.0's .mappings() returns RowMapping dicts, not tuples.
Update assertions to compare against dicts with column names as keys.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Annotate compatibility dict in writers.py to accept dialect-specific
  types like JSON and BIT
- Use separate variable names for postgres and mysql insert statements
  to avoid type conflicts
- Remove unused type: ignore comment from Checkpoint class
- Cast pagination_mode to str for enum indexing
- Assert dbapi_connection is not None before accessing autocommit

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kaapstorm
Copy link
Copy Markdown
Contributor Author

Please excuse the rebase + force pushes. I think it makes for easier reviewing than a merge with a bunch of fixes.

@jingcheng16
Copy link
Copy Markdown
Contributor

@kaapstorm Is this just a rebase because the base branch changed? Or is there any new commits that requires review?

@kaapstorm
Copy link
Copy Markdown
Contributor Author

any new commits that requires review?

Yes, to make it easier to review, fixes related to upserts were squashed into what is now commit 7c6b5d3, and commits bb8f042 and 087260b are new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants