In the REST API, each series can have one or more results. Right now
results are synthesized from message properties which are set by
the applier-report and tester-report API. In order to simplify the
conversion to REST of those APIs (e.g. using POST, PUT or PATCH
requests), move the results out of the SeriesSerializer and into
their own collection.
For now, the ViewSet is not model-based, but I am including
paginator-like fields so that it should be possible to change it
to a ModelViewSet in the future.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
api/models.py | 7 +++--
api/rest.py | 72 ++++++++++++++++++++++++++++++++++++++++++++-------
api/urls.py | 5 ++++
mods/git.py | 17 ++++++------
mods/testing.py | 10 +++----
tests/test_git.py | 44 +++++++++++++++++++------------
tests/test_rest.py | 6 +++++
tests/test_testing.py | 33 ++++++++++++-----------
8 files changed, 133 insertions(+), 61 deletions(-)
diff --git a/api/models.py b/api/models.py
index 9b52693..6141ac1 100644
--- a/api/models.py
+++ b/api/models.py
@@ -595,14 +595,13 @@ class Module(models.Model):
def __str__(self):
return self.name
-class Result(namedtuple("Result", "name status log_url data")):
+class Result(namedtuple("Result", "name status log_url obj data")):
__slots__ = ()
- def __new__(cls, name, status, log_url=None, data=None, request=None):
+ def __new__(cls, name, status, obj, log_url=None, data=None, request=None):
if log_url is not None and request is not None:
log_url = request.build_absolute_uri(log_url)
if status not in ('pending', 'success', 'failure'):
raise ValueError("invalid value '%s' for status field" % status)
return super(cls, Result).__new__(cls, status=status, log_url=log_url,
- data=data, name=name)
-
+ obj=obj, data=data, name=name)
diff --git a/api/rest.py b/api/rest.py
index 0af1f30..1e2b774 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -8,17 +8,21 @@
# This work is licensed under the MIT License. Please see the LICENSE file or
# http://opensource.org/licenses/MIT.
+from collections import OrderedDict
from django.contrib.auth.models import User
+from django.http import Http404
from django.template import loader
from mod import dispatch_module_hook
from .models import Project, Message
from .search import SearchEngine
-from rest_framework import permissions, serializers, viewsets, filters, mixins, renderers
+from rest_framework import (permissions, serializers, viewsets, filters,
+ mixins, generics, renderers)
from rest_framework.decorators import detail_route
-from rest_framework.fields import SerializerMethodField
+from rest_framework.fields import SerializerMethodField, CharField, JSONField
from rest_framework.relations import HyperlinkedIdentityField
from rest_framework.response import Response
+import rest_framework
SEARCH_PARAM = 'q'
@@ -163,8 +167,8 @@ class SeriesSerializer(BaseMessageSerializer):
resource_uri = HyperlinkedMessageField(view_name='series-detail')
message = HyperlinkedMessageField(view_name='messages-detail')
+ results = HyperlinkedMessageField(view_name='results-list', lookup_field='series_message_id')
total_patches = SerializerMethodField()
- results = SerializerMethodField()
def __init__(self, *args, **kwargs):
self.detailed = kwargs.pop('detailed', False)
@@ -177,13 +181,6 @@ class SeriesSerializer(BaseMessageSerializer):
fields=fields, detailed=self.detailed)
return fields
- def get_results(self, message):
- results = []
- request = self.context['request']
- dispatch_module_hook("rest_results_hook", request=request,
- message=message, results=results)
- return {x.name: x._asdict() for x in results}
-
def get_total_patches(self, obj):
return obj.get_total_patches()
@@ -316,3 +313,58 @@ class MessagesViewSet(ProjectMessagesViewSetMixin,
serializer = BaseMessageSerializer(page, many=True,
context=self.get_serializer_context())
return self.get_paginated_response(serializer.data)
+
+# Results
+
+class HyperlinkedResultField(HyperlinkedIdentityField):
+ def get_url(self, result, view_name, request, format):
+ obj = result.obj
+ kwargs = {'projects_pk': obj.project_id, 'series_message_id': obj.message_id,
+ 'name': result.name}
+ return self.reverse(view_name, kwargs=kwargs, request=request, format=format)
+
+class ResultSerializer(serializers.Serializer):
+ resource_uri = HyperlinkedResultField(view_name='results-detail')
+ name = CharField()
+ status = CharField() # one of 'failure', 'success', 'pending'
+ log_url = CharField(required=False)
+ data = JSONField(required=False)
+
+class SeriesResultsViewSet(viewsets.ViewSet, generics.GenericAPIView):
+ serializer_class = ResultSerializer
+ lookup_field = 'name'
+ lookup_value_regex = '[^/]+'
+
+ def get_queryset(self):
+ return Message.objects.filter(project=self.kwargs['projects_pk'],
+ message_id=self.kwargs['series_message_id'])
+
+ def get_results(self):
+ queryset = self.get_queryset()
+ try:
+ obj = queryset[0]
+ except IndexError:
+ raise Http404
+ results = []
+ dispatch_module_hook("rest_results_hook", request=self.request,
+ obj=obj, results=results)
+ return {x.name: x for x in results}
+
+ def list(self, request, *args, **kwargs):
+ results = self.get_results().values()
+ serializer = self.get_serializer(results, many=True)
+ # Fake paginator response for forwards-compatibility, in case
+ # this ViewSet becomes model-based
+ return Response(OrderedDict([
+ ('count', len(results)),
+ ('results', serializer.data)
+ ]))
+
+ def retrieve(self, request, name, *args, **kwargs):
+ results = self.get_results()
+ try:
+ result = results[name]
+ except KeyError:
+ raise Http404
+ serializer = self.get_serializer(result)
+ return Response(serializer.data)
diff --git a/api/urls.py b/api/urls.py
index 0a447f0..becabeb 100644
--- a/api/urls.py
+++ b/api/urls.py
@@ -37,11 +37,16 @@ projects_router.include_format_suffixes = False
projects_router.register('series', rest.ProjectSeriesViewSet, base_name='series')
projects_router.register('messages', rest.MessagesViewSet, base_name='messages')
+results_router = NestedDefaultRouter(projects_router, 'series', lookup='series', trailing_slash=True)
+results_router.include_format_suffixes = False
+results_router.register('results', rest.SeriesResultsViewSet, base_name='results')
+
schema_view = get_schema_view(title='API schema')
urlpatterns = _build_urls() + [
url(r"v1/", include(router.urls)),
url(r"v1/", include(projects_router.urls)),
+ url(r"v1/", include(results_router.urls)),
url(r'^v1/schema/$', schema_view),
# Use the base class's handler by default
url(r".*", views.APIView.as_view())
diff --git a/mods/git.py b/mods/git.py
index bf812df..13c3642 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -111,22 +111,23 @@ class GitModule(PatchewModule):
raise Exception("Project git repo invalid: %s" % project_git)
return upstream, branch
- def rest_results_hook(self, request, message, results):
- l = message.get_property("git.apply-log")
+ def rest_results_hook(self, request, obj, results):
+ log = obj.get_property("git.apply-log")
data = None
- if l:
- if message.get_property("git.apply-failed"):
+ if log:
+ if obj.get_property("git.apply-failed"):
status = 'failure'
else:
- git_repo = message.get_property("git.repo")
- git_tag = message.get_property("git.tag")
+ git_repo = obj.get_property("git.repo")
+ git_tag = obj.get_property("git.tag")
data = {'repo': git_repo, 'tag': 'refs/tags/' + git_tag}
status = 'success'
- log_url = reverse("git-log", kwargs={'series': message.message_id})
+ log_url = reverse("git-log", kwargs={'series': obj.message_id})
else:
status = 'pending'
log_url = None
- results.append(Result(name='git', status=status, log_url=log_url, data=data,
+ results.append(Result(name='git', obj=obj, status=status,
+ log_url=log_url, data=data,
request=request))
def prepare_message_hook(self, request, message, detailed):
diff --git a/mods/testing.py b/mods/testing.py
index cf22004..21942d5 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -276,19 +276,19 @@ class TestingModule(PatchewModule):
return reverse("testing-get-prop",
kwargs={"project_or_series": obj.message_id})
- def rest_results_hook(self, request, message, results):
- for pn, p in message.get_properties().items():
+ def rest_results_hook(self, request, obj, results):
+ for pn, p in obj.get_properties().items():
if not pn.startswith("testing.report."):
continue
tn = pn[len("testing.report."):]
failed = not p["passed"]
- log_url = self.reverse_testing_log(message, tn, request=request, html=False)
+ log_url = self.reverse_testing_log(obj, tn, request=request, html=False)
passed_str = "failure" if failed else "success"
data = p.copy()
del data['passed']
- results.append(Result(name='testing.' + tn, status=passed_str, log_url=log_url,
- request=request, data=data))
+ results.append(Result(name='testing.' + tn, obj=obj, status=passed_str,
+ log_url=log_url, request=request, data=data))
def prepare_message_hook(self, request, message, detailed):
if not message.is_series_head:
diff --git a/tests/test_git.py b/tests/test_git.py
index c2c98ae..8bff7d5 100755
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -100,31 +100,41 @@ class GitTest(PatchewTestCase):
self.repo + " patchew/20160628014747.20971-2-famz@redhat.com")
def test_rest_need_apply(self):
- resp = self.apply_and_retrieve("0013-foo-patch.mbox.gz", self.p.id,
- "20160628014747.20971-1-famz@redhat.com")
- self.assertEqual(resp.data['results']['git']['status'], 'pending')
+ self.cli_import("0013-foo-patch.mbox.gz")
+ MESSAGE_ID = '20160628014747.20971-1-famz@redhat.com'
+ resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['status'], 'pending')
+ self.assertEqual(resp.data['log_url'], None)
def test_rest_apply_failure(self):
self.cli_import("0014-bar-patch.mbox.gz")
self.do_apply()
- resp = self.api_client.get(self.REST_BASE + 'series/?q=project:QEMU')
- self.assertEqual(resp.data['results'][0]['is_complete'], True)
- self.assertEqual(resp.data['results'][0]['results']['git']['status'], 'failure')
- self.assertEqual('repo' in resp.data['results'][0]['results']['git'], False)
- self.assertEqual('tag' in resp.data['results'][0]['results']['git'], False)
- log = self.client.get(resp.data['results'][0]['results']['git']['log_url'])
- self.assertEquals(log.status_code, 200)
+ MESSAGE_ID = '20160628014747.20971-2-famz@redhat.com'
+ resp = self.api_client.get('%sseries/%s/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['is_complete'], True)
+
+ resp = self.api_client.get('%sseries/%s/results/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['results'][0]['name'], 'git')
+
+ resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['status'], 'failure')
+ self.assertEqual('repo' in resp.data, False)
+ self.assertEqual('tag' in resp.data, False)
+ log = self.client.get(resp.data['log_url'])
+ self.assertEqual(log.status_code, 200)
def test_rest_apply_success(self):
self.cli_import("0013-foo-patch.mbox.gz")
self.do_apply()
- resp = self.api_client.get(self.REST_BASE + 'series/?q=project:QEMU')
- self.assertEqual(resp.data['results'][0]['is_complete'], True)
- self.assertEqual(resp.data['results'][0]['results']['git']['status'], 'success')
- self.assertEqual(resp.data['results'][0]['results']['git']['data']['repo'], self.repo)
- self.assertEqual(resp.data['results'][0]['results']['git']['data']['tag'], "refs/tags/patchew/20160628014747.20971-1-famz@redhat.com")
- log = self.client.get(resp.data['results'][0]['results']['git']['log_url'])
- self.assertEquals(log.status_code, 200)
+ MESSAGE_ID = '20160628014747.20971-1-famz@redhat.com'
+ resp = self.api_client.get('%sseries/%s/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['is_complete'], True)
+ resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+ self.assertEqual(resp.data['status'], 'success')
+ self.assertEqual(resp.data['data']['repo'], self.repo)
+ self.assertEqual(resp.data['data']['tag'], "refs/tags/patchew/20160628014747.20971-1-famz@redhat.com")
+ log = self.client.get(resp.data['log_url'])
+ self.assertEqual(log.status_code, 200)
if __name__ == '__main__':
main()
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 28ca10b..391714f 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -189,6 +189,12 @@ class RestTest(PatchewTestCase):
resp = self.api_client.get(self.REST_BASE + 'projects/12345/series/')
self.assertEqual(resp.data['count'], 0)
+ def test_series_results_list(self):
+ resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
+ self.p.id, '20160628014747.20971-1-famz@redhat.com')
+ resp = self.api_client.get(resp1.data['results'])
+ self.assertEqual(resp.data['count'], len(resp.data['results']))
+
def test_series_search(self):
resp1 = self.apply_and_retrieve('0004-multiple-patch-reviewed.mbox.gz',
self.p.id, '1469192015-16487-1-git-send-email-berrange@redhat.com')
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 0897bbc..369a806 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -93,41 +93,40 @@ class TestingTest(PatchewTestCase):
capabilities=[])
self.assertFalse(td)
- def test_rest_basic(self):
- resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
- self.assertEquals('testing.tests' in resp.data['results'], False)
-
def test_rest_done_success(self):
self.msg_testing_done(log='everything good!', passed=True)
- resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
- self.assertEquals(resp.data['results']['testing.tests']['status'], 'success')
- log = self.client.get(resp.data['results']['testing.tests']['log_url'])
+ resp = self.api_client.get('%sseries/%s/results/testing.tests/' % (self.PROJECT_BASE, self.msg.message_id))
+ self.assertEquals(resp.data['status'], 'success')
+ log = self.client.get(resp.data['log_url'])
self.assertEquals(log.status_code, 200)
self.assertEquals(log.content, b'everything good!')
def test_rest_done_failure(self):
- self.msg_testing_done(log='sorry no good', passed=False)
- resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
- self.assertEquals(resp.data['results']['testing.tests']['status'], 'failure')
- log = self.client.get(resp.data['results']['testing.tests']['log_url'])
+ self.msg_testing_done(log='sorry no good', passed=False, random_stuff='xyz')
+ resp = self.api_client.get('%sseries/%s/results/testing.tests/' % (self.PROJECT_BASE, self.msg.message_id))
+ self.assertEquals(resp.data['status'], 'failure')
+ self.assertEquals(resp.data['data']['random_stuff'], 'xyz')
+ log = self.client.get(resp.data['log_url'])
self.assertEquals(log.status_code, 200)
self.assertEquals(log.content, b'sorry no good')
def test_api_report_success(self):
self.api_login()
msgid = self.msg_testing_report(log='everything good!', passed=True)
- resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
- self.assertEquals(resp.data['results']['testing.a']['status'], 'success')
- log = self.client.get(resp.data['results']['testing.a']['log_url'])
+ resp = self.api_client.get('%sseries/%s/results/testing.a/' % (self.PROJECT_BASE, self.msg.message_id))
+ self.assertEquals(resp.data['data']['is_timeout'], False)
+ self.assertEquals(resp.data['status'], 'success')
+ log = self.client.get(resp.data['log_url'])
self.assertEquals(log.status_code, 200)
self.assertEquals(log.content, b'everything good!')
def test_api_report_failure(self):
self.api_login()
msgid = self.msg_testing_report(log='sorry no good', passed=False)
- resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
- self.assertEquals(resp.data['results']['testing.a']['status'], 'failure')
- log = self.client.get(resp.data['results']['testing.a']['log_url'])
+ resp = self.api_client.get('%sseries/%s/results/testing.a/' % (self.PROJECT_BASE, self.msg.message_id))
+ self.assertEquals(resp.data['data']['is_timeout'], False)
+ self.assertEquals(resp.data['status'], 'failure')
+ log = self.client.get(resp.data['log_url'])
self.assertEquals(log.status_code, 200)
self.assertEquals(log.content, b'sorry no good')
--
2.16.2
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
On Sat, 03/24 14:37, Paolo Bonzini wrote:
> In the REST API, each series can have one or more results. Right now
> results are synthesized from message properties which are set by
> the applier-report and tester-report API. In order to simplify the
> conversion to REST of those APIs (e.g. using POST, PUT or PATCH
> requests), move the results out of the SeriesSerializer and into
> their own collection.
>
> For now, the ViewSet is not model-based, but I am including
> paginator-like fields so that it should be possible to change it
> to a ModelViewSet in the future.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> api/models.py | 7 +++--
> api/rest.py | 72 ++++++++++++++++++++++++++++++++++++++++++++-------
> api/urls.py | 5 ++++
> mods/git.py | 17 ++++++------
> mods/testing.py | 10 +++----
> tests/test_git.py | 44 +++++++++++++++++++------------
> tests/test_rest.py | 6 +++++
> tests/test_testing.py | 33 ++++++++++++-----------
> 8 files changed, 133 insertions(+), 61 deletions(-)
>
> diff --git a/api/models.py b/api/models.py
> index 9b52693..6141ac1 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -595,14 +595,13 @@ class Module(models.Model):
> def __str__(self):
> return self.name
>
> -class Result(namedtuple("Result", "name status log_url data")):
> +class Result(namedtuple("Result", "name status log_url obj data")):
> __slots__ = ()
>
> - def __new__(cls, name, status, log_url=None, data=None, request=None):
> + def __new__(cls, name, status, obj, log_url=None, data=None, request=None):
> if log_url is not None and request is not None:
> log_url = request.build_absolute_uri(log_url)
> if status not in ('pending', 'success', 'failure'):
> raise ValueError("invalid value '%s' for status field" % status)
> return super(cls, Result).__new__(cls, status=status, log_url=log_url,
> - data=data, name=name)
> -
> + obj=obj, data=data, name=name)
> diff --git a/api/rest.py b/api/rest.py
> index 0af1f30..1e2b774 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -8,17 +8,21 @@
> # This work is licensed under the MIT License. Please see the LICENSE file or
> # http://opensource.org/licenses/MIT.
>
> +from collections import OrderedDict
> from django.contrib.auth.models import User
> +from django.http import Http404
> from django.template import loader
>
> from mod import dispatch_module_hook
> from .models import Project, Message
> from .search import SearchEngine
> -from rest_framework import permissions, serializers, viewsets, filters, mixins, renderers
> +from rest_framework import (permissions, serializers, viewsets, filters,
> + mixins, generics, renderers)
> from rest_framework.decorators import detail_route
> -from rest_framework.fields import SerializerMethodField
> +from rest_framework.fields import SerializerMethodField, CharField, JSONField
> from rest_framework.relations import HyperlinkedIdentityField
> from rest_framework.response import Response
> +import rest_framework
>
> SEARCH_PARAM = 'q'
>
> @@ -163,8 +167,8 @@ class SeriesSerializer(BaseMessageSerializer):
>
> resource_uri = HyperlinkedMessageField(view_name='series-detail')
> message = HyperlinkedMessageField(view_name='messages-detail')
> + results = HyperlinkedMessageField(view_name='results-list', lookup_field='series_message_id')
DRF novice question, why isn't this 'HyperlinkedResultField'?
> total_patches = SerializerMethodField()
> - results = SerializerMethodField()
Fam
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
On 30/03/2018 08:28, Fam Zheng wrote: >> resource_uri = HyperlinkedMessageField(view_name='series-detail') >> message = HyperlinkedMessageField(view_name='messages-detail') >> + results = HyperlinkedMessageField(view_name='results-list', lookup_field='series_message_id') > > DRF novice question, why isn't this 'HyperlinkedResultField'? All that the "HyperlinkedMessageField" is to ensure that reverse() gets both the project primary key and the message id. The actual URL that is generated depends on the view_name. There is a generic subclass of HyperlinkedIdentityField in the drf-nested-routers package, that would avoid the need for this. However, I couldn't get it to work. :) Paolo _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
© 2016 - 2024 Red Hat, Inc.