refactor: set cascade_backrefs=False for SqlaTable#42213
Conversation
Enable the pytest error filter for the "'SqlaTable' object is being merged into a Session along the backref cascade path" RemovedIn20Warning and adopt the SQLAlchemy 2.0 behavior for the Database.tables backref. Constructing SqlaTable(database=...) no longer implicitly merges the new object into the Database's session. All persisting code paths already add tables to the session explicitly. See discussion #40273. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review Agent Run #d9b62eActionable 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42213 +/- ##
==========================================
- Coverage 65.19% 65.19% -0.01%
==========================================
Files 2768 2768
Lines 156081 156082 +1
Branches 35719 35719
==========================================
- Hits 101754 101751 -3
- Misses 52365 52368 +3
- Partials 1962 1963 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
With cascade_backrefs=False, assigning `tbl.database = database` no longer implicitly adds a transient SqlaTable to the session. In generic_loader.py, `fetch_metadata()` and the outer caller each call `db.session.merge(tbl)`; previously both calls resolved to the same already-pending object (added via the backref cascade), but now each merge() creates a separate transient copy, producing two pending inserts for the same uuid and a uq_tables_uuid violation on the very first example loaded (which then poisons the session for every subsequent example in the same transaction). Also add the missing explicit `db.session.add()` in two sqla_models_tests.py tests that relied on the same implicit cascade to persist a freshly constructed SqlaTable before committing/deleting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit touched generic_loader.py, so CI pylint now lints the whole file and flags two pre-existing session.commit() calls that lack the @transaction decorator. These are example seed-loader scripts, not command-layer units of work; use the inline disable convention already established in security/manager.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review Agent Run #4aa873Actionable 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 |
1 participant
See #40273
SUMMARY
Enable errors for the
"SqlaTable" object is being merged into a Session along the backref cascade pathRemovedIn20Warning, and adopt the SQLAlchemy 2.0 behavior for theDatabase.tablesbackref.Constructing
SqlaTable(database=...)no longer implicitly merges the new object into the Database's session. All code paths that persist datasets already callsession.add()explicitly (dataset commands, uploaders, importers), so no call sites needed changes.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
🤖 Generated with Claude Code