From b459c933951ecb0614a367f2a85cc55b17678554 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Fri, 30 May 2014 08:30:08 -0700 Subject: [PATCH] Handle missing commits If gertty is interrupted when fetching commits, or, well, any number of weird things happen to the git repo, we could end up not having the needed commits locally. Do a sanity check before opening a change and fetch them if needed. Change-Id: I9bbecd09d1820e405b51a8471c5fd9e4fe8a7841 --- gertty/app.py | 7 ++++++- gertty/gitrepo.py | 9 +++++++++ gertty/sync.py | 34 ++++++++++++++++++---------------- gertty/view/__init__.py | 16 ++++++++++++++++ gertty/view/change.py | 25 +++++++++++++++++++++++++ gertty/view/change_list.py | 7 ++++++- 6 files changed, 80 insertions(+), 18 deletions(-) diff --git a/gertty/app.py b/gertty/app.py index f27d11b..65a2e15 100644 --- a/gertty/app.py +++ b/gertty/app.py @@ -28,6 +28,7 @@ from gertty import mywid from gertty import sync from gertty.view import project_list as view_project_list from gertty.view import change as view_change +import gertty.view WELCOME_TEXT = """\ Welcome to Gertty! @@ -226,7 +227,11 @@ class App(object): change_key = change and change.key or None if change_key is None: return self.error('Change is not in local database.') - self.changeScreen(view_change.ChangeView(self, change_key)) + try: + view = view_change.ChangeView(self, change_key) + self.changeScreen(view) + except gertty.view.DisplayError as e: + self.app.error(e.message) def error(self, message): dialog = mywid.MessageDialog('Error', message) diff --git a/gertty/gitrepo.py b/gertty/gitrepo.py index 2806c16..53082d2 100644 --- a/gertty/gitrepo.py +++ b/gertty/gitrepo.py @@ -18,6 +18,7 @@ import os import re import git +import gitdb OLD = 0 NEW = 1 @@ -150,6 +151,14 @@ class Repo(object): if not os.path.exists(path): git.Repo.clone_from(self.url, self.path) + def hasCommit(self, sha): + repo = git.Repo(self.path) + try: + repo.commit(sha) + except gitdb.exc.BadObject: + return False + return True + def fetch(self, url, refspec): repo = git.Repo(self.path) try: diff --git a/gertty/sync.py b/gertty/sync.py index b523c6a..9f02dd6 100644 --- a/gertty/sync.py +++ b/gertty/sync.py @@ -148,7 +148,7 @@ class SyncProjectTask(Task): # in the db optionally we could sync all changes ever change = session.getChangeByID(c['id']) if change or (c['status'] not in self._closed_statuses): - sync.submitTask(SyncChangeTask(c['id'], self.priority)) + sync.submitTask(SyncChangeTask(c['id'], priority=self.priority)) self.log.debug("Change %s update %s" % (c['id'], c['updated'])) class SyncChangeByCommitTask(Task): @@ -167,7 +167,7 @@ class SyncChangeByCommitTask(Task): self.log.debug('Query: %s ' % (query,)) with app.db.getSession() as session: for c in changes: - sync.submitTask(SyncChangeTask(c['id'], self.priority)) + sync.submitTask(SyncChangeTask(c['id'], priority=self.priority)) self.log.debug("Sync change %s for its commit %s" % (c['id'], self.commit)) class SyncChangeByNumberTask(Task): @@ -187,15 +187,16 @@ class SyncChangeByNumberTask(Task): self.log.debug('Query: %s ' % (query,)) with app.db.getSession() as session: for c in changes: - task = SyncChangeTask(c['id'], self.priority) + task = SyncChangeTask(c['id'], priority=self.priority) self.tasks.append(task) sync.submitTask(task) self.log.debug("Sync change %s because it is number %s" % (c['id'], self.number)) class SyncChangeTask(Task): - def __init__(self, change_id, priority=NORMAL_PRIORITY): + def __init__(self, change_id, force_fetch=False, priority=NORMAL_PRIORITY): super(SyncChangeTask, self).__init__(priority) self.change_id = change_id + self.force_fetch = force_fetch def __repr__(self): return '' % (self.change_id,) @@ -228,18 +229,19 @@ class SyncChangeTask(Task): new_revision = False for remote_commit, remote_revision in remote_change.get('revisions', {}).items(): revision = session.getRevisionByCommit(remote_commit) - if not revision: - # TODO: handle multiple parents - url = sync.app.config.url + change.project.name - if 'anonymous http' in remote_revision['fetch']: - ref = remote_revision['fetch']['anonymous http']['ref'] - else: - ref = remote_revision['fetch']['http']['ref'] - url = list(urlparse.urlsplit(url)) - url[1] = '%s:%s@%s' % (sync.app.config.username, - sync.app.config.password, url[1]) - url = urlparse.urlunsplit(url) + # TODO: handle multiple parents + url = sync.app.config.url + change.project.name + if 'anonymous http' in remote_revision['fetch']: + ref = remote_revision['fetch']['anonymous http']['ref'] + else: + ref = remote_revision['fetch']['http']['ref'] + url = list(urlparse.urlsplit(url)) + url[1] = '%s:%s@%s' % (sync.app.config.username, + sync.app.config.password, url[1]) + url = urlparse.urlunsplit(url) + if (not revision) or self.force_fetch: fetches.append((url, ref)) + if not revision: revision = change.createRevision(remote_revision['_number'], remote_revision['commit']['message'], remote_commit, remote_revision['commit']['parents'][0]['commit']) @@ -419,7 +421,7 @@ class UploadReviewTask(Task): # Inside db session for rollback sync.post('changes/%s/revisions/%s/review' % (change.id, revision.commit), data) - sync.submitTask(SyncChangeTask(change.id, self.priority)) + sync.submitTask(SyncChangeTask(change.id, priority=self.priority)) class Sync(object): def __init__(self, app): diff --git a/gertty/view/__init__.py b/gertty/view/__init__.py index e69de29..3cfc7e0 100644 --- a/gertty/view/__init__.py +++ b/gertty/view/__init__.py @@ -0,0 +1,16 @@ +# Copyright 2014 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +class DisplayError(Exception): + pass diff --git a/gertty/view/change.py b/gertty/view/change.py index 167b7f3..661d7b0 100644 --- a/gertty/view/change.py +++ b/gertty/view/change.py @@ -20,6 +20,7 @@ from gertty import gitrepo from gertty import mywid from gertty import sync from gertty.view import diff as view_diff +import gertty.view class ReviewDialog(urwid.WidgetWrap): signals = ['save', 'cancel'] @@ -330,8 +331,32 @@ This Screen self.listbox.body.append(urwid.Divider()) self.listbox_patchset_start = len(self.listbox.body) + self.checkGitRepo() self.refresh() + def checkGitRepo(self): + missing_revisions = False + change_number = None + change_id = None + with self.app.db.getSession() as session: + change = session.getChange(self.change_key) + change_number = change.number + change_id = change.id + repo = self.app.getRepo(change.project.name) + for revision in change.revisions: + if not (repo.hasCommit(revision.parent) and + repo.hasCommit(revision.commit)): + missing_revisions = True + break + if missing_revisions: + self.app.log.warning("Missing some commits for change %s" % change_number) + task = sync.SyncChangeTask(change_id, force_fetch=True, + priority=sync.HIGH_PRIORITY) + self.app.sync.submitTask(task) + succeeded = task.wait(300) + if not succeeded: + raise gertty.view.DisplayError("Git commits not present in local repository") + def refresh(self): change_info = [] with self.app.db.getSession() as session: diff --git a/gertty/view/change_list.py b/gertty/view/change_list.py index 6cec924..b4bfba6 100644 --- a/gertty/view/change_list.py +++ b/gertty/view/change_list.py @@ -16,6 +16,7 @@ import urwid from gertty import mywid from gertty.view import change as view_change +import gertty.view class ChangeRow(urwid.Button): change_focus_map = {None: 'focused', @@ -154,4 +155,8 @@ This Screen return super(ChangeListView, self).keypress(size, key) def onSelect(self, button, change_key): - self.app.changeScreen(view_change.ChangeView(self.app, change_key)) + try: + view = view_change.ChangeView(self.app, change_key) + self.app.changeScreen(view) + except gertty.view.DisplayError as e: + self.app.error(e.message)