#191 db models: replace backref= with back_populates=
Closed 2 years ago by kparal. Opened 2 years ago by kparal.

@@ -23,6 +23,20 @@ 

  import re

  from typing import Any

  

+ from blockerbugs import db

+ 

+ update_fixes: Any = db.Table(

+     'update_fixes',

+     db.Column('bug_id', db.Integer, db.ForeignKey('bug.id')),

+     db.Column('update_id', db.Integer, db.ForeignKey('update.id'))

+ )

+ 

+ update_milestones: Any = db.Table(

+     'update_milestones',

+     db.Column('milestone_id', db.Integer, db.ForeignKey('milestone.id')),

+     db.Column('update_id', db.Integer, db.ForeignKey('update.id'))

+ )

+ 

  

  class EnumSymbol(object):

      """Define a fixed symbol tied to a parent class."""

file modified
+13 -8
@@ -19,13 +19,13 @@ 

  

  """ORM class for bug information stored in the database"""

  

- from blockerbugs import db, BaseModel

  import datetime

  import json

  from typing import Any, Optional

  

- # unused imports, but required by dynamic relationships:

- from blockerbugs.models.milestone import Milestone

+ from blockerbugs import db, BaseModel, models

+ import blockerbugs.models.milestone as model_milestone

+ import blockerbugs.models.update as model_update

  

  

  class Bug(BaseModel):
@@ -63,8 +63,8 @@ 

      accepted_fe = db.Column(db.Boolean)

      prioritized = db.Column(db.Boolean)

      milestone_id = db.Column(db.Integer, db.ForeignKey('milestone.id'))

-     milestone: Optional[Milestone] = db.relationship(

-         'Milestone', backref=db.backref('bugs', lazy='dynamic'))

+     milestone: Optional['model_milestone.Milestone'] = db.relationship(

+         'Milestone', back_populates='bugs')

      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
@@ -77,6 +77,9 @@ 

      ```"""

      _depends_on: str = db.Column('depends_on', db.Text, default='[]', nullable=False)

      """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=(model_update.Update.status.desc(), model_update.Update.pending.desc()))

  

      def __init__(self,

                   bugid: Optional[int],
@@ -84,7 +87,7 @@ 

                   summary: Optional[str],

                   status: Optional[str],

                   component: Optional[str],

-                  milestone: Optional[Milestone],

+                  milestone: Optional['model_milestone.Milestone'],

                   active: Optional[bool],

                   needinfo: Optional[bool],

                   needinfo_requestee: Optional[str],
@@ -126,7 +129,8 @@ 

      def __repr__(self):

          return '<bug %d: %s>' % (self.bugid, self.summary)

  

-     def update(self, buginfo: dict, tracker_type: str, milestone: Milestone) -> None:

+     def update(self, buginfo: dict, tracker_type: str, milestone: 'model_milestone.Milestone'

+                ) -> None:

          """Update this Bug with new values. See `Bug.from_data()` for the description of

          arguments.

          """
@@ -163,7 +167,8 @@ 

                               'as separate instances!')

  

      @classmethod

-     def from_data(cls, buginfo: dict[str, Any], milestone: Milestone, tracker_type: str) -> 'Bug':

+     def from_data(cls, buginfo: dict[str, Any], milestone: 'model_milestone.Milestone',

+                   tracker_type: str) -> 'Bug':

          """Create a `Bug` instance from the provided arguments

  

          :param buginfo: for a definition of this dict, see `BugSync.extract_information()`

@@ -19,9 +19,11 @@ 

  

  """Database model for release criteria"""

  

- from blockerbugs import db, BaseModel

  import datetime

  

+ from blockerbugs import db, BaseModel

+ import blockerbugs.models.milestone as model_milestone

+ 

  

  class Criterion(BaseModel):

      id = db.Column(db.Integer, primary_key=True)
@@ -29,6 +31,7 @@ 

      criterion = db.Column(db.String(4096), unique=False)

      current = db.Column(db.Boolean)

      milestone_id = db.Column(db.Integer, db.ForeignKey('milestone.id'))

+     milestone: 'model_milestone.Milestone' = db.relationship('Milestone', back_populates='criteria')

      number = db.Column(db.Integer, unique=False)

  

      def __init__(self, criterion, milestone, number,

file modified
+15 -11
@@ -19,12 +19,11 @@ 

  

  """ORM class for release milestone info stored in the database"""

  

- from blockerbugs import db, BaseModel

  from typing import Any, Optional

  

- # unused imports, but required by dynamic relationships:

- from blockerbugs.models.criterion import Criterion

- from blockerbugs.models.release import Release

+ from blockerbugs import db, BaseModel, models

+ from blockerbugs.models import bug, criterion, update

+ from blockerbugs.models import release as model_release

  

  

  class Milestone(BaseModel):
@@ -34,8 +33,8 @@ 

  

      id = db.Column(db.Integer, primary_key=True)

      release_id = db.Column(db.Integer, db.ForeignKey('release.id'))

-     release: Optional[Release] = db.relationship(

-         'Release', backref=db.backref('milestones', lazy='dynamic'))

+     release: 'Optional[model_release.Release]' = db.relationship(

+         'Release', back_populates='milestones')

      version = db.Column(db.String(80))

      """E.g. 'beta' or 'final'"""

      name = db.Column(db.String(80), unique=True)
@@ -51,14 +50,19 @@ 

      current = db.Column(db.Boolean)

      """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."""

-     criteria: list[Criterion] = db.relationship('Criterion', backref='milestone', lazy='dynamic')

+     bugs: list['bug.Bug'] = db.relationship('Bug', back_populates='milestone', lazy='dynamic')

+     updates: list['update.Update'] = db.relationship('Update', secondary=models.update_milestones,

+                                                      back_populates='milestones')

+     criteria: list['criterion.Criterion'] = db.relationship('Criterion', back_populates='milestone',

+                                                             lazy='dynamic')

      succeeds_id = db.Column(db.Integer, db.ForeignKey('milestone.id'), nullable=True)

-     succeeds = db.relationship(

-         'Milestone', backref=db.backref('succeeded_by', remote_side=[id], uselist=False),

-         lazy='dynamic')

+     succeeds: 'Milestone' = db.relationship('Milestone', back_populates='succeeded_by',

+                                             lazy='dynamic')

+     succeeded_by: 'Milestone' = db.relationship('Milestone', back_populates='succeeds',

+                                                 remote_side=[id], uselist=False)

  

      def __init__(self,

-                  release: Optional[Release],

+                  release: Optional['model_release.Release'],

                   version: Optional[str],

                   blocker_tracker: Optional[int],

                   fe_tracker: Optional[int],

@@ -19,10 +19,12 @@ 

  

  """Database model for releases"""

  

- from blockerbugs import db, BaseModel

  from datetime import datetime

  from typing import Optional

  

+ from blockerbugs import db, BaseModel

+ from blockerbugs.models import milestone, update

+ 

  

  class Release(BaseModel):

      """This represents an OS release, primarily determined by its number."""
@@ -36,6 +38,10 @@ 

      """This is used for past (historical) releases. If this is `True`, all discussion tickets (as in

      `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')

+     updates: list['update.Update'] = db.relationship('Update', back_populates='release',

+                                                      lazy='dynamic')

  

      def __init__(self, number: int, active: Optional[bool] = True,

                   discussions_closed: Optional[bool] = False) -> None:

file modified
+16 -36
@@ -19,25 +19,9 @@ 

  

  """Database model for Bodhi updates"""

  

- from blockerbugs import db, BaseModel

- 

- # imports required by dynamic relationships:

- from blockerbugs.models.bug import Bug

- from blockerbugs.models.release import Release  # noqa: F401

- from blockerbugs.models.milestone import Milestone  # noqa: F401

- 

- 

- update_fixes = db.Table(

-     'update_fixes',

-     db.Column('bug_id', db.Integer, db.ForeignKey('bug.id')),

-     db.Column('update_id', db.Integer, db.ForeignKey('update.id'))

- )

- 

- update_milestones = db.Table(

-     'update_milestones',

-     db.Column('milestone_id', db.Integer, db.ForeignKey('milestone.id')),

-     db.Column('update_id', db.Integer, db.ForeignKey('update.id'))

- )

+ from blockerbugs import db, BaseModel, models

+ from blockerbugs.models import bug, milestone

+ from blockerbugs.models import release as model_release

  

  

  class Update(BaseModel):
@@ -53,16 +37,12 @@ 

      date_pushed_testing = db.Column(db.DateTime, nullable=True)

      date_pushed_stable = db.Column(db.DateTime, nullable=True)

      date_obsoleted = db.Column(db.DateTime, nullable=True)

-     bugs = db.relationship('Bug',

-                            secondary=update_fixes,

-                            backref=db.backref('updates',

-                                               lazy='dynamic',

-                                               order_by=(status.desc(), pending.desc())))

+     bugs: list['bug.Bug'] = db.relationship(

+         'Bug', secondary=models.update_fixes, back_populates='updates')

      release_id = db.Column(db.Integer, db.ForeignKey('release.id'))

-     release = db.relationship('Release',

-                               backref=db.backref('bugs', lazy='dynamic'))

-     milestones = db.relationship('Milestone', secondary=update_milestones,

-                                  backref='updates')

+     release: 'model_release.Release' = db.relationship('Release', back_populates='updates')

+     milestones: list['milestone.Milestone'] = db.relationship(

+         'Milestone', secondary=models.update_milestones, back_populates='updates')

  

      def __init__(self, title, url, karma, status, bugs, release,

                   milestones, stable_karma=3, date_submitted=None):
@@ -96,15 +76,15 @@ 

          if updateinfo['date_pushed_stable']:

              self.date_pushed_stable = updateinfo['date_pushed_stable']

          self.pending = updateinfo['pending']

-         current_bugkeys = [(bug.bugid, bug.milestone) for bug in self.bugs]

+         current_bugkeys = [(currentbug.bugid, currentbug.milestone) for currentbug in self.bugs]

          for bugid in updateinfo['bugs']:

-             newbugs = Bug.query.filter_by(bugid=bugid).all()

-             for bug in newbugs:

-                 if not (bug.bugid, bug.milestone) in current_bugkeys:

-                     self.bugs.append(bug)

-                     current_bugkeys.append((bug.bugid, bug.milestone))

-                 if bug.milestone and bug.milestone not in self.milestones:

-                     self.milestones.append(bug.milestone)

+             newbugs = bug.Bug.query.filter_by(bugid=bugid).all()

+             for newbug in newbugs:

+                 if not (newbug.bugid, newbug.milestone) in current_bugkeys:

+                     self.bugs.append(newbug)

+                     current_bugkeys.append((newbug.bugid, newbug.milestone))

+                 if newbug.milestone and newbug.milestone not in self.milestones:

+                     self.milestones.append(newbug.milestone)

  

      @classmethod

      def from_data(cls, updateinfo, release):

The advantage of back_populates= is that it is defined on both sides of the
relationship. Therefore when you look at a class, it is clear which attributes
it has, no attributes are created silently by a completely different class/
module. You can also define additional parameters like lazy and order_by
on the correct side of the relationship, rather than have it reversed.

The downside of defining the relationship on both sides is that it's much easier
to stumble into a circular import. I had to rework the imports to only import
a module (instead of a particular class), and all typehints had to be defined
as strings. The association tables had to be moved into an independent module
(I decided to use __init__.py).


@jskladan I'd very appreciate if you could look at this patch and tell me if the change seems sane to you, or if there's some gotcha in changing backref to back_populates that I haven't thought of. The test suite passes and the app runs, so it seems to work exactly as before, but I'm really not sure whether there could be some other consequences. Thanks a lot.

Build succeeded.

rebased onto 0c6ff08

2 years ago

Build succeeded.

I received a "shruggie ok" from @jskladan, I consider it good enough :-)

Pull-Request has been closed by kparal

2 years ago