[Patchew-devel] [PATCH 01/10] models: move Result log_url from constructor to renderer

Paolo Bonzini pbonzini at redhat.com
Wed Jun 13 10:37:35 UTC 2018


The log_url is currently provided at construction time.  However, the
URL is not going to be stored in the database and the constructor concept
will disappear altogether once Results will be fetched from the database.

In preparation for changing Result to a model, retrieve it through a
method in the renderer.  This has other benefits too.  It also removes
the request argument parameter from the Result constructor (which
would not have any equivalent for database-stored Results) and from the
rest_results_hook; and it makes log_url a read-only field in the REST API,
which will be the right thing once PUT of Results will be supported.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 api/models.py   | 16 +++++++++++-----
 api/rest.py     |  9 ++++++---
 mods/git.py     | 15 ++++++++-------
 mods/testing.py | 19 +++++++++++--------
 www/views.py    |  4 ++--
 5 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/api/models.py b/api/models.py
index b4f8468..1753c6b 100644
--- a/api/models.py
+++ b/api/models.py
@@ -645,7 +645,7 @@ class Module(models.Model):
     def __str__(self):
         return self.name
 
-class Result(namedtuple("Result", "name status log log_url obj data renderer")):
+class Result(namedtuple("Result", "name status log obj data renderer")):
     __slots__ = ()
     PENDING = 'pending'
     SUCCESS = 'success'
@@ -653,12 +653,10 @@ class Result(namedtuple("Result", "name status log log_url obj data renderer")):
     RUNNING = 'running'
     VALID_STATUSES = (PENDING, SUCCESS, FAILURE, RUNNING)
 
-    def __new__(cls, name, status, obj, log=None, log_url=None, data=None, request=None, renderer=None):
-        if log_url is not None and request is not None:
-            log_url = request.build_absolute_uri(log_url)
+    def __new__(cls, name, status, obj, log=None, data=None, renderer=None):
         if status not in cls.VALID_STATUSES:
             raise ValueError("invalid value '%s' for status field" % status)
-        return super(cls, Result).__new__(cls, status=status, log=log, log_url=log_url,
+        return super(cls, Result).__new__(cls, status=status, log=log,
                                           obj=obj, data=data, name=name, renderer=renderer)
 
     def is_success(self):
@@ -680,3 +678,11 @@ class Result(namedtuple("Result", "name status log log_url obj data renderer")):
         if self.renderer is None:
             return None
         return self.renderer.render_result(self)
+
+    def get_log_url(self, request=None):
+        if not self.is_completed() or self.renderer is None:
+            return None
+        log_url = self.renderer.get_result_log_url(self)
+        if log_url is not None and request is not None:
+            log_url = request.build_absolute_uri(log_url)
+        return log_url
diff --git a/api/rest.py b/api/rest.py
index 084dbbc..f853964 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -450,9 +450,13 @@ class ResultSerializer(serializers.Serializer):
     resource_uri = HyperlinkedResultField(view_name='results-detail')
     name = CharField()
     status = CharField() # one of 'failure', 'success', 'pending', 'running'
-    log_url = CharField(required=False)
+    log_url = SerializerMethodField(required=False)
     data = JSONField(required=False)
 
+    def get_log_url(self, obj):
+        request = self.context['request']
+        return obj.get_log_url(request)
+
 class ResultSerializerFull(ResultSerializer):
     log = CharField(required=False)
 
@@ -473,8 +477,7 @@ class ResultsViewSet(viewsets.ViewSet, generics.GenericAPIView):
         except IndexError:
             raise Http404
         results = []
-        dispatch_module_hook("rest_results_hook", request=self.request,
-                             obj=obj, results=results,
+        dispatch_module_hook("rest_results_hook", obj=obj, results=results,
                              detailed=detailed)
         return {x.name: x for x in results}
 
diff --git a/mods/git.py b/mods/git.py
index fe10423..4bfb5c6 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -126,7 +126,7 @@ class GitModule(PatchewModule):
     def rest_series_fields_hook(self, request, fields, detailed):
         fields['based_on'] = PluginMethodField(obj=self, required=False)
 
-    def rest_results_hook(self, request, obj, results, detailed=False):
+    def rest_results_hook(self, obj, results, detailed=False):
         if not isinstance(obj, Message):
             return
         log = obj.get_property("git.apply-log")
@@ -148,13 +148,10 @@ class GitModule(PatchewModule):
                     if git_base:
                         data['base'] = git_base
                 status = Result.SUCCESS
-            log_url = reverse("git-log", kwargs={'series': obj.message_id})
         else:
             status = Result.PENDING
-            log_url = None
         results.append(Result(name='git', obj=obj, status=status,
-                              log=log, log_url=log_url, data=data,
-                              request=request, renderer=self))
+                              log=log, data=data, renderer=self))
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
@@ -199,9 +196,10 @@ class GitModule(PatchewModule):
         if not result.is_completed():
             return None
 
-        html_log_url = result.log_url + "?html=1"
+        log_url = result.get_log_url()
+        html_log_url = log_url + "?html=1"
         colorbox_a = format_html('<a class="cbox-log" data-link="{}" href="{}">apply log</a>',
-                                 html_log_url, result.log_url)
+                                 html_log_url, log_url)
         if result.is_failure():
             return format_html('Failed in applying to current master ({})', colorbox_a)
         else:
@@ -218,6 +216,9 @@ class GitModule(PatchewModule):
                 s += format_html('<br/><samp>git fetch {} {}</samp>', git_repo, git_tag)
             return s
 
+    def get_result_log_url(self, result):
+        return reverse("git-log", kwargs={'series': result.obj.message_id})
+
     def prepare_project_hook(self, request, project):
         if not project.maintained_by(request.user):
             return
diff --git a/mods/testing.py b/mods/testing.py
index 824df15..efa2d82 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -165,8 +165,8 @@ class TestingModule(PatchewModule):
                                       "testing_name": test}) + "?type=project"
         if html:
             log_url += "&html=1"
-        # Generate a full URL, including the host and port, for use in email
-        # notifications and REST API responses.
+        # Generate a full URL, including the host and port, for use in
+        # email notifications
         if request:
             log_url = request.build_absolute_uri(log_url)
         return log_url
@@ -251,7 +251,7 @@ class TestingModule(PatchewModule):
                         })
         return ret
 
-    def rest_results_hook(self, request, obj, results, detailed=False):
+    def rest_results_hook(self, obj, results, detailed=False):
         all_tests = set([k for k, v in _instance.get_tests(obj).items() if v["enabled"]])
         for pn, p in obj.get_properties().items():
             if not pn.startswith("testing.report."):
@@ -262,7 +262,6 @@ class TestingModule(PatchewModule):
             except:
                 pass
             failed = not p["passed"]
-            log_url = self.reverse_testing_log(obj, tn, request=request, html=False)
             passed_str = Result.FAILURE if failed else Result.SUCCESS
             if detailed:
                 log = obj.get_property("testing.log." + tn)
@@ -272,8 +271,7 @@ class TestingModule(PatchewModule):
             data = p.copy()
             del data['passed']
             results.append(Result(name='testing.' + tn, obj=obj, status=passed_str,
-                                  log=log, log_url=log_url, request=request, data=data,
-                                  renderer=self))
+                                  log=log, data=data, renderer=self))
 
         if obj.get_property("testing.ready"):
             for tn in all_tests:
@@ -305,15 +303,20 @@ class TestingModule(PatchewModule):
                 "char": "T",
                 })
 
+    def get_result_log_url(self, result):
+        tn = result.name[len("testing."):]
+        return self.reverse_testing_log(result.obj, tn, html=False)
+
     def render_result(self, result):
         if not result.is_completed():
             return None
         pn = result.name
         tn = pn[len("testing."):]
-        html_log_url = result.log_url + '&html=1'
+        log_url = result.get_log_url()
+        html_log_url = log_url + '&html=1'
         passed_str = "failed" if result.is_failure() else "passed"
         return format_html('Test <b>{}</b> <a class="cbox-log" data-link="{}" href="{}">{}</a>',
-                           tn, html_log_url, result.log_url, passed_str)
+                           tn, html_log_url, log_url, passed_str)
 
     def check_active_testers(self, project):
         at = []
diff --git a/www/views.py b/www/views.py
index 1eb6c3c..c2010c5 100644
--- a/www/views.py
+++ b/www/views.py
@@ -93,8 +93,8 @@ def prepare_series(request, s, skip_patches=False):
 
 def prepare_results(request, obj):
     results = []
-    dispatch_module_hook("rest_results_hook", request=request,
-                         obj=obj, results=results, detailed=False)
+    dispatch_module_hook("rest_results_hook", obj=obj,
+                         results=results, detailed=False)
 
     results_dicts = []
     for result in results:
-- 
2.17.0





More information about the Patchew-devel mailing list