PR#264: Fix warnings about SQLAlchemy 2.0 - fedora-qa/blockerbugs

fedora-qa / blockerbugs

#264 Fix warnings about SQLAlchemy 2.0

Closed 2 years ago by kparal. Opened 2 years ago by kparal.

sqlalchemy2 into develop

Fix warnings about SQLAlchemy 2.0

Kamil Páral • 2 years ago

f46b988

blockerbugs/models/bug.py

file modified

+13 -2

		`@@ -67,7 +67,7 @@`
		`prioritized = db.Column(db.Boolean(create_constraint=True, name='prioritized_bool'))`
		`milestone_id = db.Column(db.Integer, db.ForeignKey('milestone.id'))`
		`milestone: Optional['model_milestone.Milestone'] = db.relationship(`
		`- 'Milestone', back_populates='bugs')`
		`+ 'Milestone', back_populates='bugs', cascade_backrefs=False)`
		`discussion_link = db.Column(db.String, unique=False)`
		`votes: str = db.Column(db.Text, default='{}', nullable=False)`
		`"""A JSON representation of dict of all BugVoteTrackers and corresponding output of`
		`@@ -82,7 +82,7 @@`
		`"""A JSON list of bug numbers which this bug depends on"""`
		`updates: list['model_update.Update'] = db.relationship(`
		`'Update', secondary=models.update_fixes, back_populates='bugs', lazy='dynamic',`
		`- order_by=('[Update.status.desc(), Update.request.desc()]')`
		`+ order_by=('[Update.status.desc(), Update.request.desc()]'), cascade_backrefs=False`
		`)`

		`def __init__(self,`
		`@@ -209,3 +209,14 @@`
		`newbug = Bug(buginfo['bug_id'], '', '', '', '', milestone, buginfo['active'], False, '')`
		`newbug.update(buginfo, tracker_type, milestone)`
		`return newbug`
		`+`
		`+`
		`+ def known_bugs(bugids: list[int]) -> list['Bug']:`
		+ """Return `Bug` instances for all bugs which match IDs present in `bugids`. If some bug ID isn't
		`+ known (not present in DB), it is skipped.`
		`+ """`
		`+ bugs = []`
		`+ for bugid in set(bugids): # deduplicate just to be sure`
		`+ assert isinstance(bugid, int)`
		`+ bugs.extend(Bug.query.filter_by(bugid=bugid).all())`
		`+ return bugs`

blockerbugs/models/milestone.py

file modified

+6 -4

		`@@ -34,7 +34,7 @@`
		`id = db.Column(db.Integer, primary_key=True)`
		`release_id = db.Column(db.Integer, db.ForeignKey('release.id'))`
		`release: 'Optional[model_release.Release]' = db.relationship(`
		`- 'Release', back_populates='milestones')`
		`+ 'Release', back_populates='milestones', cascade_backrefs=False)`
		`version = db.Column(db.String(80))`
		`"""E.g. 'beta' or 'final'"""`
		`name = db.Column(db.String(80), unique=True)`
		`@@ -50,12 +50,14 @@`
		`current = db.Column(db.Boolean(create_constraint=True, name='current_bool'))`
		`"""Current milestone is the most relevant one currently. Usually it is the nearest milestone`
		`in the future. There should be at most one milestone marked as current."""`
		`- bugs: list['bug.Bug'] = db.relationship('Bug', back_populates='milestone', lazy='dynamic')`
		`+ bugs: list['bug.Bug'] = db.relationship('Bug', back_populates='milestone', lazy='dynamic',`
		`+ cascade_backrefs=False)`
		`succeeds_id = db.Column(db.Integer, db.ForeignKey('milestone.id'), nullable=True)`
		`succeeds: 'Milestone' = db.relationship('Milestone', back_populates='succeeded_by',`
		`- lazy='dynamic')`
		`+ lazy='dynamic', cascade_backrefs=False)`
		`succeeded_by: 'Milestone' = db.relationship('Milestone', back_populates='succeeds',`
		`- remote_side=[id], uselist=False)`
		`+ remote_side=[id], uselist=False,`
		`+ cascade_backrefs=False)`

		`def __init__(self,`
		`release: Optional['model_release.Release'],`

blockerbugs/models/release.py

file modified

+2 -2

		`@@ -40,9 +40,9 @@`
		`Bug.discussion_link`) have been closed ("cleaned up")."""
		`last_update_sync: datetime = db.Column(db.DateTime)`
		`milestones: list['milestone.Milestone'] = db.relationship('Milestone', back_populates='release',`
		`- lazy='dynamic')`
		`+ lazy='dynamic', cascade_backrefs=False)`
		`updates: list['update.Update'] = db.relationship('Update', back_populates='release',`
		`- lazy='dynamic')`
		`+ lazy='dynamic', cascade_backrefs=False)`

		`def __init__(self, number: int, active: Optional[bool] = True,`
		`discussions_closed: Optional[bool] = False) -> None:`

blockerbugs/models/update.py

file modified

+23 -9

		`@@ -39,7 +39,8 @@`
		`updateid: str = db.Column(db.Text, nullable=False, unique=True)`
		`"""E.g. FEDORA-2021-5282b5cafd"""`
		`release_id: int = db.Column(db.Integer, db.ForeignKey('release.id'), nullable=False)`
		`- release: 'model_release.Release' = db.relationship('Release', back_populates='updates')`
		`+ release: 'model_release.Release' = db.relationship('Release', back_populates='updates',`
		`+ cascade_backrefs=False)`
		`"""The release this update was created for in Bodhi"""`
		`status: str = db.Column(db.Enum('stable', 'pending', 'testing', create_constraint=True,`
		`name='update_status_enum'),`
		`@@ -63,7 +64,7 @@`
		`stable_karma: Optional[int] = db.Column(db.Integer)`
		`"""Target karma value for allowing the update to go stable (via auto or manual push)."""`
		`bugs: list['bug.Bug'] = db.relationship(`
		`- 'Bug', secondary=models.update_fixes, back_populates='updates')`
		`+ 'Bug', secondary=models.update_fixes, back_populates='updates', cascade_backrefs=False)`
		`"""A list of bugs this update claims to fix and we track them"""`

		`_tmpstr = 'A placeholder value when creating incomplete Update objects'`
		`@@ -93,7 +94,17 @@`
		`def __repr__(self) -> str:`
		`return f'<Update(id={self.id},updateid={self.updateid})>'`

		`- def sync(self, updateinfo: dict[str, Any]) -> None:`
		`+ def sync(self, updateinfo: dict[str, Any], bugs: list['bug.Bug'] = []) -> None:`
		+ """Update the existing `Update` instance with a fresh data from an `updateinfo` dictionary
		`+ (coming from Bodhi).`
		`+`
		`+ :param updateinfo: a dictionary describing a Bodhi update, see`
		+ `util.update_sync.extract_information()`
		+ :param bugs: A list of 'Bug' instances which should be associated with this `Update`.
		+ Normally this would be retrieved from `updateinfo['bugs']`, but in certain
		`+ cases you don't want to run DB queries at the moment (e.g. during a new`
		+ `Update` initialization) and want to run it beforehand and provide it here.
		`+ """`
		`self.updateid = updateinfo['updateid']`
		`self.status = updateinfo['status']`
		`self.karma = updateinfo['karma']`
		`@@ -104,11 +115,10 @@`
		`self.stable_karma = updateinfo['stable_karma']`

		`self.bugs.clear()`
		`- for bugid in set(updateinfo['bugs']): # deduplicate just to be sure`
		`- assert isinstance(bugid, int)`
		`- tracked_bugs = bug.Bug.query.filter_by(bugid=bugid).all()`
		`- for tracked_bug in tracked_bugs:`
		`- self.bugs.append(tracked_bug)`
		`+ if bugs:`
		`+ self.bugs.extend(bugs)`
		`+ else:`
		`+ self.bugs = bug.known_bugs(updateinfo['bugs'])`

		`# a quick check that mandatory values seem initialized`
		`checkfields = [self.updateid, self.release, self.status, self.karma, self.url,`
		`@@ -118,6 +128,10 @@`

		`@classmethod`
		`def from_data(cls, updateinfo: dict[str, Any], release: 'model_release.Release') -> 'Update':`
		+ # retrieve `Bug` instances beforehand, otherwise SQLAlchemy complains when a DB query is
		+ # performed and at the same time a new `Update` gets associated with existing objects while
		`+ # not yet added to a DB session`
		`+ bugs = bug.known_bugs(updateinfo['bugs'])`
		`newupdate = Update(updateid=updateinfo['updateid'],`
		`release=release,`
		`status=cls._tmpstr,`
		`@@ -128,5 +142,5 @@`
		`title=cls._tmpstr,`
		`stable_karma=None,`
		`bugs=[])`
		`- newupdate.sync(updateinfo)`
		`+ newupdate.sync(updateinfo, bugs)`
		`return newupdate`

kparal commented 2 years ago

SQLAlchemy 1.4 started emitting these warnings:

Object is being merged into a Session along the backref cascade

Here's some background:
https://docs.sqlalchemy.org/en/14/errors.html#error-s9r1
https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#change-5150

This patch adds cascade_backrefs=False into all db.relationship, which will
be the only supported behavior in SQLAlchemy 2.0. However, that started emitting
yet another warning:

Object of type not in session, add operation along <B.a> will not proceed

This happens when a new object with bi-directional relationships is created (in
our case Update), it's not yet added to a DB session, but the relationships are
set with objects already in the DB and then autoflush is triggered, e.g. by
running a DB query. The solution is to avoid running DB queries while the object
is being created (and therefore the caller couldn't add it yet to the DB
session). Make the DB queries beforehand.