From 795de4235d3b0df05be4658871744ef2e4e0c0a1 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 28 Mar 2014 14:20:06 -0400 Subject: [PATCH 1/3] Change "cannot connect to redid" to be a raised exception --- data/buildlogs.py | 6 ++++-- endpoints/api/build.py | 12 ++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/data/buildlogs.py b/data/buildlogs.py index 817fbc2b4..43723e211 100644 --- a/data/buildlogs.py +++ b/data/buildlogs.py @@ -1,6 +1,8 @@ import redis import json +class BuildStatusRetrievalError(Exception): + pass class BuildLogs(object): ERROR = 'error' @@ -45,7 +47,7 @@ class BuildLogs(object): log_entries = self._redis.lrange(self._logs_key(build_id), start_index, -1) return (llen, (json.loads(entry) for entry in log_entries)) except redis.ConnectionError: - return (0, []) + raise BuildStatusRetrievalError('Cannot retrieve build logs') @staticmethod def _status_key(build_id): @@ -65,6 +67,6 @@ class BuildLogs(object): try: fetched = self._redis.get(self._status_key(build_id)) except redis.ConnectionError: - return None + raise BuildStatusRetrievalError('Cannot retrieve build status') return json.loads(fetched) if fetched else None diff --git a/endpoints/api/build.py b/endpoints/api/build.py index f14d097bb..0e3ef76e7 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -11,6 +11,7 @@ from endpoints.common import start_build from endpoints.trigger import BuildTrigger from data import model from auth.permissions import ModifyRepositoryPermission +from data.buildlogs import BuildStatusRetrievalError logger = logging.getLogger(__name__) @@ -48,7 +49,11 @@ def trigger_view(trigger): def build_status_view(build_obj, can_write=False): - status = build_logs.get_status(build_obj.uuid) + try: + status = build_logs.get_status(build_obj.uuid) + except BuildStatusRetrievalError: + status = None + logger.debug('Can write: %s job_config: %s', can_write, build_obj.job_config) resp = { 'id': build_obj.uuid, @@ -169,7 +174,10 @@ class RepositoryBuildLogs(RepositoryParamResource): start = int(request.args.get('start', 0)) - count, logs = build_logs.get_log_entries(build.uuid, start) + try: + count, logs = build_logs.get_log_entries(build.uuid, start) + except BuildStatusRetrievalError: + count, logs = (0, []) response_obj.update({ 'start': start, From abfc38f10a42464766bc714f5c9a8679f0483403 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 28 Mar 2014 14:42:29 -0400 Subject: [PATCH 2/3] Really fix the build status --- endpoints/api/build.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/endpoints/api/build.py b/endpoints/api/build.py index 0e3ef76e7..63c85ad8e 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -49,15 +49,17 @@ def trigger_view(trigger): def build_status_view(build_obj, can_write=False): + phase = build_obj.phase try: status = build_logs.get_status(build_obj.uuid) except BuildStatusRetrievalError: - status = None + status = {} + phase = 'cannot_load' logger.debug('Can write: %s job_config: %s', can_write, build_obj.job_config) resp = { 'id': build_obj.uuid, - 'phase': build_obj.phase if status else 'cannot_load', + 'phase': phase, 'started': format_date(build_obj.started), 'display_name': build_obj.display_name, 'status': status or {}, From 6fd2440294c232f85f6eb5ccc2d8a5eac7eec624 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 28 Mar 2014 15:32:56 -0400 Subject: [PATCH 3/3] Handle empty GitHub repositories and do not 500 if the repository cannot be read --- endpoints/api/trigger.py | 10 ++++++++-- endpoints/trigger.py | 12 +++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index 1eb7cd169..c28188057 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -13,7 +13,8 @@ from endpoints.api.build import (build_status_view, trigger_view, RepositoryBuil get_trigger_config) from endpoints.common import start_build from endpoints.trigger import (BuildTrigger as BuildTriggerBase, TriggerDeactivationException, - TriggerActivationException, EmptyRepositoryException) + TriggerActivationException, EmptyRepositoryException, + RepositoryReadException) from data import model from auth.permissions import UserAdminPermission @@ -116,9 +117,14 @@ class BuildTriggerSubdirs(RepositoryParamResource): 'status': 'success' } except EmptyRepositoryException as exc: + return { + 'status': 'success', + 'subdir': [] + } + except RepositoryReadException as exc: return { 'status': 'error', - 'message': exc.msg + 'message': exc.message } else: raise Unauthorized() diff --git a/endpoints/trigger.py b/endpoints/trigger.py index 82a3284ab..873f51c9b 100644 --- a/endpoints/trigger.py +++ b/endpoints/trigger.py @@ -38,6 +38,9 @@ class ValidationRequestException(Exception): class EmptyRepositoryException(Exception): pass +class RepositoryReadException(Exception): + pass + class BuildTrigger(object): def __init__(self): @@ -209,9 +212,12 @@ class GithubBuildTrigger(BuildTrigger): return [os.path.dirname(elem.path) for elem in commit_tree.tree if (elem.type == u'blob' and os.path.basename(elem.path) == u'Dockerfile')] - except GithubException: - msg = 'Unable to list contents of repository: %s' % source - raise EmptyRepositoryException(msg) + except GithubException as ge: + message = ge.data.get('message', 'Unable to list contents of repository: %s' % source) + if message == 'Branch not found': + raise EmptyRepositoryException() + + raise RepositoryReadException(message) @staticmethod def _prepare_build(config, repo, commit_sha, build_name, ref):