Skip to content

Commit

Permalink
Remove redundant obsolete_if_unstable
Browse files Browse the repository at this point in the history
`obsolete_if_unstable` was introduced by fedora-infra#901 to cover one path:
obsoleting a 'pending' update if it reached its negative karma
threshold. At the time, `check_karma_thresholds` only reached
its negative karma check for updates in 'testing' state, it did
not run on updates in 'pending' state.

However, fedora-infra#1556 extended `check_karma_thresholds` to also run on
updates in 'pending' state, which actually rendered
`obsolete_if_unstable` entirely redundant, except in one case -
calling `comment()` with `check_karma` set to `False` - which
was introduced solely to make it possible to test
`obsolete_if_unstable`.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
AdamWill committed Apr 4, 2024
1 parent c080bfb commit 8738094
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 32 deletions.
25 changes: 3 additions & 22 deletions bodhi-server/bodhi/server/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3530,28 +3530,12 @@ def update_bugs(self, bug_ids, session):
session.flush()
return new

def obsolete_if_unstable(self, db):
"""
Obsolete the update if it reached the negative karma threshold while pending.
Args:
db (sqlalchemy.orm.session.Session): A database session.
"""
if self.autokarma and self.status is UpdateStatus.pending \
and self.request is UpdateRequest.testing\
and self.karma <= self.unstable_karma:
log.info("%s has reached unstable karma thresholds", self.alias)
self.obsolete(db)
log.debug("%s has been obsoleted.", self.alias)
return

def comment(self, session, text, karma=0, author=None, karma_critpath=0,
bug_feedback=None, testcase_feedback=None, check_karma=True,
email_notification=True):
bug_feedback=None, testcase_feedback=None, email_notification=True):
"""Add a comment to this update.
If the karma reaches the 'stable_karma' value, then request that this update be marked
as stable. If it reaches the 'unstable_karma', it is unpushed.
as stable. If it reaches the 'unstable_karma' value, then unpush it (obsolete it).
"""
if not author:
raise ValueError('You must provide a comment author')
Expand Down Expand Up @@ -3606,7 +3590,7 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,

log.info("Updated %s karma to %d", self.alias, self.karma)

if check_karma and author not in config.get('system_users'):
if author not in config.get('system_users'):
try:
self.check_karma_thresholds(session, 'bodhi')
except LockedUpdateException:
Expand All @@ -3620,9 +3604,6 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
'name': 'karma', 'description': str(e),
})

# Obsolete pending update if it reaches unstable karma threshold
self.obsolete_if_unstable(session)

session.flush()

for feedback_dict in bug_feedback:
Expand Down
10 changes: 0 additions & 10 deletions bodhi-server/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3633,16 +3633,6 @@ def test_last_modified_no_dates(self):
self.obj.last_modified
assert 'Update has no timestamps set:' in str(exc.value)

def test_obsolete_if_unstable_unstable(self):
"""Test obsolete_if_unstable() when all conditions are met for instability."""
self.obj.autokarma = True
self.obj.status = UpdateStatus.pending
self.obj.request = UpdateRequest.testing
self.obj.unstable_karma = -1
self.obj.comment(self.db, 'foo', -1, 'foo', check_karma=False)

assert self.obj.status == UpdateStatus.obsolete

@mock.patch('bodhi.server.models.log.warning')
def test_remove_tag_emptystring(self, warning):
"""Test remove_tag() with a tag of ''."""
Expand Down

0 comments on commit 8738094

Please sign in to comment.