fix(tests): drop removed subtransactions= kwarg from Session.begin() - #42866
fix(tests): drop removed subtransactions= kwarg from Session.begin()#42866rusackas wants to merge 1 commit into
Conversation
TestDatasource.setUp explicitly opened a transaction with db.session.begin(subtransactions=True) before each test, relying on tearDown's rollback() to isolate them. subtransactions was already deprecated in SQLAlchemy 1.4 and is removed outright in 2.0 (TypeError: unexpected keyword argument 'subtransactions'), surfacing as a failure while investigating discussion #40273's SQLAlchemy 2.0 bump. The explicit begin() is unnecessary either way: Session autobegins on first use under both 1.4 and 2.0, so tearDown's rollback() still correctly discards whatever the test did without it.
Code Review Agent Run #1fb897Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes an explicit SQLAlchemy transaction begin in TestDatasource.setUp that relies on the removed subtransactions= kwarg, keeping test isolation via tearDown() rollback.
Changes:
- Drops
db.session.begin(subtransactions=True)fromTestDatasourcesetup to avoid SQLAlchemy 2.0TypeError. - Continues to rely on
db.session.rollback()intearDown()for per-test isolation.
SQLAlchemy 2.0 removed the subtransactions= parameter from Session.begin(); TestDatasource.setUp() called it unconditionally, failing every test in the class with "TypeError: scoped_session.begin() got an unexpected keyword argument 'subtransactions'" before the test body ever ran. Same fix as #42866 (open at the time of this commit, not yet merged to master despite this branch already having merged master's tip) - applying it directly here since it's blocking all further local verification of tests/integration_tests/datasource_tests.py and is independent of the rest of this branch's SQLAlchemy 2.0 work. Once #42866 merges to master, a future master-merge into this branch will no-op on this file. Verified locally (sqlite): all 31 tests in datasource_tests.py pass.
SUMMARY
TestDatasource.setUp(tests/integration_tests/datasource_tests.py) explicitly opened a transaction withdb.session.begin(subtransactions=True)before each test, relying ontearDown'srollback()to isolate them.subtransactions=was already deprecated in SQLAlchemy 1.4 and is removed outright in 2.0 (TypeError: scoped_session.begin() got an unexpected keyword argument 'subtransactions'). It surfaced while investigating discussion #40273's SQLAlchemy 2.0 bump, but it's a standalone fix independent of that work —Sessionautobegins on first use under both 1.4 and 2.0, so the explicitbegin()call is unnecessary either way, andtearDown'srollback()still correctly discards whatever the test did without it.TESTING INSTRUCTIONS
Ran the full file locally against sqlite (with Redis up for the two cache-dependent tests):
ADDITIONAL INFORMATION