From 364e185412eebbbd80a6861fcdf70640222018d0 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Wed, 16 Jul 2014 12:29:36 -0400 Subject: [PATCH] Fix up how the pager view works and how we return errors from some views so that we can propogate non 200 statuses --- awx/api/utils/decorators.py | 46 +++++++++++++++++++++---------------- awx/api/views.py | 26 ++++++++++++++------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/awx/api/utils/decorators.py b/awx/api/utils/decorators.py index 730abbf6f8..bbaa20e828 100644 --- a/awx/api/utils/decorators.py +++ b/awx/api/utils/decorators.py @@ -7,6 +7,7 @@ import functools from rest_framework.response import Response from rest_framework.settings import api_settings +from rest_framework import status def paginated(method): """Given an method with a Django REST Framework API method signature @@ -46,29 +47,34 @@ def paginated(method): kwargs['ordering'] = ordering # Okay, call the underlying method. - results, count = method(self, request, *args, **kwargs) + results, count, stat = method(self, request, *args, **kwargs) + if stat is None: + stat = status.HTTP_200_OK - # Determine the next and previous pages, if any. - prev, next_ = None, None - if page > 1: - get_copy = copy.copy(request.GET) - get_copy['page'] = page - 1 - prev = '%s?%s' % (request.path, get_copy.urlencode()) - if count > offset + limit: - get_copy = copy.copy(request.GET) - get_copy['page'] = page + 1 - next_ = '%s?%s' % (request.path, get_copy.urlencode()) + if stat == status.HTTP_200_OK: + # Determine the next and previous pages, if any. + prev, next_ = None, None + if page > 1: + get_copy = copy.copy(request.GET) + get_copy['page'] = page - 1 + prev = '%s?%s' % (request.path, get_copy.urlencode()) + if count > offset + limit: + get_copy = copy.copy(request.GET) + get_copy['page'] = page + 1 + next_ = '%s?%s' % (request.path, get_copy.urlencode()) - # Compile the results into a dictionary with pagination - # information. - answer = collections.OrderedDict(( - ('count', count), - ('next', next_), - ('previous', prev), - ('results', results), - )) + # Compile the results into a dictionary with pagination + # information. + answer = collections.OrderedDict(( + ('count', count), + ('next', next_), + ('previous', prev), + ('results', results), + )) + else: + answer = results # Okay, we're done; return response data. - return Response(answer) + return Response(answer, status=stat) return func diff --git a/awx/api/views.py b/awx/api/views.py index 1dfbc7ce9c..19543e4577 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1630,7 +1630,10 @@ class JobJobPlaysList(BaseJobEventsList): @paginated def get(self, request, limit, offset, ordering, *args, **kwargs): all_plays = [] - job = get_object_or_404(self.parent_model, pk=self.kwargs['pk']) + job = Job.objects.filter(pk=self.kwargs['pk']) + if not job.exists(): + return ({'detail': 'job not found'}, -1, status.HTTP_404_NOT_FOUND) + job = job[0] # Put together a queryset for relevant job events. qs = job.job_events.filter(event='playbook_on_play_start') @@ -1686,7 +1689,7 @@ class JobJobPlaysList(BaseJobEventsList): all_plays.append(play_details) # Done; return the plays and the total count. - return all_plays, count + return all_plays, count, None class JobJobTasksList(BaseJobEventsList): @@ -1709,11 +1712,18 @@ class JobJobTasksList(BaseJobEventsList): # Get the job and the parent task. # If there's no event ID specified, this will return a 404. - # FIXME: Make this a good error message. - job = get_object_or_404(self.parent_model, pk=self.kwargs['pk']) - parent_task = get_object_or_404(job.job_events, - pk=int(request.QUERY_PARAMS.get('event_id', -1)), - ) + job = Job.objects.filter(pk=self.kwargs['pk']) + if not job.exists(): + return ({'detail': 'job not found'}, -1, status.HTTP_404_NOT_FOUND) + job = job[0] + + if 'event_id' not in request.QUERY_PARAMS: + return ({'detail': '"event_id" not provided'}, -1, status.HTTP_400_BAD_REQUEST) + + parent_task = job.job_events.filter(pk=int(request.QUERY_PARAMS.get('event_id', -1))) + if not parent_task.exists(): + return ({'detail': 'parent event not found'}, -1, status.HTTP_404_NOT_FOUND) + parent_task = parent_task[0] # Some events correspond to a playbook or task starting up, # and these are what we're interested in here. @@ -1818,7 +1828,7 @@ class JobJobTasksList(BaseJobEventsList): results.append(task_data) # Done; return the results and count. - return results, count + return (results, count, None) class UnifiedJobTemplateList(ListAPIView):