We found a bug in the blockerbugs app yesterday that is preventing updates from being properly synced and leaving us with an empty page of pending updates:
https://qa.fedoraproject.org/blockerbugs/milestone/21/alpha/updates
I'd like to deploy a new version of blockerbugs with the fix for that bug and adding a convenience endpoint to the API. The added API endpoint is of little to no risk because there are no current users of that API and the change itself is very small.
I'd like to get some +1s if the change is acceptable during freeze.
Tim
Reviews of changes: (bugfix) https://phab.qadevel.cloud.fedoraproject.org/D228 (endpoint) https://phab.qadevel.cloud.fedoraproject.org/D230
Full diff of code change:
diff --git a/blockerbugs/cli.py b/blockerbugs/cli.py index 2ec4898..b164b7b 100644 --- a/blockerbugs/cli.py +++ b/blockerbugs/cli.py @@ -166,7 +166,7 @@ def sync_updates(fullsync=False): active_releases = Release.query.filter_by(active=True).all() update_sync = UpdateSync(db) for release in active_releases: - update_sync.sync_updates(release) + update_sync.sync_updates(release, fullsync)
def minify(): diff --git a/blockerbugs/controllers/api/api.py b/blockerbugs/controllers/api/api.py index 09f437f..721b442 100644 --- a/blockerbugs/controllers/api/api.py +++ b/blockerbugs/controllers/api/api.py @@ -233,4 +233,7 @@ def update_spin(rel_num, milestone_version, spin_id): db.session.commit() return JsonResponse(status=200)
- +@api_v0.route('/milestones/current') +def get_current_milestone(): + current_milestone = get_or_404(Milestone, current=True) + return JsonResponse(current_milestone.simple()) diff --git a/blockerbugs/models/milestone.py b/blockerbugs/models/milestone.py index 68bbf0e..0b1739d 100644 --- a/blockerbugs/models/milestone.py +++ b/blockerbugs/models/milestone.py @@ -60,3 +60,13 @@ class Milestone(db.Model): def __repr__(self): number = self.release.number if self.release else -1 return 'milestone: %d-%s' % (number, self.version) + + def simple(self): + return {'id': self.id, + 'release': self.release.number, + 'version': self.version, + 'name': self.name, + 'blocker_tracker': self.blocker_tracker, + 'fe_tracker': self.fe_tracker, + 'current': self.current, + } diff --git a/blockerbugs/models/update.py b/blockerbugs/models/update.py index 033e623..777beec 100644 --- a/blockerbugs/models/update.py +++ b/blockerbugs/models/update.py @@ -77,7 +77,7 @@ class Update(db.Model): def __str__(self): return 'update: %s' % (self.title)
- def sync(self, updateinfo, milestone): + def sync(self, updateinfo, fullsync=False): self.title = updateinfo['title'] self.url = updateinfo['url'] self.karma = updateinfo['karma'] @@ -91,16 +91,18 @@ class Update(db.Model): self.pending = updateinfo['pending'] current_bugids = [bug.bugid for bug in self.bugs] for bugid in updateinfo['bugs']: - if not bugid in current_bugids: + # if full sync is specified, we want to sync all of the bugs + if fullsync or not bugid in current_bugids: newbugs = Bug.query.filter_by(bugid=bugid).all() for bug in newbugs: - self.bugs.append(bug) - if not milestone in self.milestones: - self.milestones.append(milestone) + if not bugid in current_bugids: + self.bugs.append(bug) + if not bug.milestone in self.milestones: + self.milestones.append(bug.milestone)
@classmethod - def from_data(cls, updateinfo, release, milestone): + def from_data(cls, updateinfo, release): newupdate = Update(updateinfo['title'], '', 0, '', [], release, None) - newupdate.sync(updateinfo, milestone) + newupdate.sync(updateinfo) return newupdate
diff --git a/blockerbugs/util/update_sync.py b/blockerbugs/util/update_sync.py index af8b2bf..4587a8c 100644 --- a/blockerbugs/util/update_sync.py +++ b/blockerbugs/util/update_sync.py @@ -132,7 +132,7 @@ class UpdateSync(object): buglist.extend(milestone.bugs.filter_by(active=True).all()) return buglist
- def sync_bug_updates(self, release, bugs): + def sync_bug_updates(self, release, bugs, fullsync=False): starttime = datetime.utcnow() bugs_ids = [bug.bugid for bug in bugs] self.log.debug('searching for updates for bugs %s' % str(bugs_ids)) @@ -154,11 +154,10 @@ class UpdateSync(object): if existing_update: self.log.debug( 'syncing existing update %s' % existing_update.title) - existing_update.sync(update, bug.milestone) + existing_update.sync(update, fullsync) else: self.log.debug('creating new update %s' % update['title']) - existing_update = Update.from_data(update, release, - bug.milestone) + existing_update = Update.from_data(update, release) self.db.session.add(existing_update) self.db.session.commit()
@@ -166,7 +165,7 @@ class UpdateSync(object): self.db.session.add(release) self.db.session.commit()
- def sync_updates(self, release): + def sync_updates(self, release, fullsync=False): bugs = self.get_release_bugs(release) self.log.info( 'found %d bugs in f%d release' % (len(bugs), release.number)) @@ -175,4 +174,4 @@ class UpdateSync(object): 'no bugs in f%d release, skip update' % (release.number)) release.last_update_sync = datetime.utcnow() return - self.sync_bug_updates(release, bugs) + self.sync_bug_updates(release, bugs, fullsync) diff --git a/testing/test_api.py b/testing/test_api.py index 34c90e3..30f20f6 100644 --- a/testing/test_api.py +++ b/testing/test_api.py @@ -24,6 +24,8 @@ class TestRestAPI(object): cls.release = add_release(99) cls.milestone = add_milestone(cls.release, 'final', 100, 101, '99-final', True) + cls.milestone.current = True + cls.milestone2 = add_milestone(cls.release, 'beta', 200, 201, '99-beta') bug1 = add_bug(9000, 'testbug1', cls.milestone) @@ -287,3 +289,10 @@ class TestRestAPI(object): assert resp.status_code == httplib.FORBIDDEN error = json.loads(resp.data)['error'] assert error['code'] == errors.AuthFailedError.code + + def test_get_current_milestone(self): + url = '/api/v0/milestones/current' + resp = self.client.get(url) + assert resp.status_code == httplib.OK + data = json.loads(resp.data) + assert data['name'] == self.milestone.name
On Tue, Sep 09, 2014 at 08:58:58AM -0600, Tim Flink wrote:
+@api_v0.route('/milestones/current') +def get_current_milestone():
- current_milestone = get_or_404(Milestone, current=True)
- return JsonResponse(current_milestone.simple())
- def sync(self, updateinfo, milestone):
- def sync(self, updateinfo, fullsync=False):
@classmethod
- def from_data(cls, updateinfo, release, milestone):
- def from_data(cls, updateinfo, release):
These are the two places which scare me a little since you are replacing an argument with another.
So assuming you have checked that the function calls were updated everywhere to the new behavior/arguments, +1 for me.
Pierre
infrastructure@lists.fedoraproject.org