[Patchew-devel] [PATCH 05/10] git: switch to Result model

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


Operate on a Result named 'git' instead of using multiple properties.
Only the hook has to convert the Result to namedtuple format, until the
REST API and web views are converted to access the database directly.
---
 api/blobs.py                                |   8 ++
 api/migrations/0028_populate_git_results.py | 104 ++++++++++++++++
 api/migrations/__init__.py                  |  49 ++++++++
 mods/git.py                                 | 124 +++++++++-----------
 tests/test_git.py                           |  34 +++---
 5 files changed, 235 insertions(+), 84 deletions(-)
 create mode 100644 api/migrations/0028_populate_git_results.py

diff --git a/api/blobs.py b/api/blobs.py
index b1c7d34..20d4567 100644
--- a/api/blobs.py
+++ b/api/blobs.py
@@ -35,3 +35,11 @@ def load_blob_json(name):
     except json.decoder.JSONDecodeError as e:
         logging.error('Failed to load blob %s: %s' %(name, e))
         return None
+
+def delete_blob(name):
+    fn = os.path.join(settings.DATA_DIR, "blob", name + ".xz")
+    try:
+        os.remove(fn)
+    except FileNotFoundError:
+        pass
+
diff --git a/api/migrations/0028_populate_git_results.py b/api/migrations/0028_populate_git_results.py
new file mode 100644
index 0000000..482641a
--- /dev/null
+++ b/api/migrations/0028_populate_git_results.py
@@ -0,0 +1,104 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations
+from django.db.models import Count
+from api.migrations import get_property, set_property, delete_property_blob
+
+import datetime
+import lzma
+
+# For Result's constant status values
+import api.models
+
+def result_from_properties(apps, schema_editor):
+    # We can't import the models directly as they may be a newer
+    # version than this migration expects. We use the historical version.
+    # The code is mostly based on the implementation of the old
+    # "rest_results_hook" method in GitModule, which used to build
+    # a Result namedtuple from the properties
+    Message = apps.get_model('api', 'Message')
+    MessageProperty = apps.get_model('api', 'MessageProperty')
+    MessageResult = apps.get_model('api', 'MessageResult')
+    LogEntry = apps.get_model('api', 'LogEntry')
+    messages = Message.objects.filter(properties__name__startswith='git.').distinct()
+    for m in messages:
+        need_apply = get_property(MessageProperty, 'git.need-apply', message=m)
+        if need_apply is None:
+            continue
+        log = get_property(MessageProperty, "git.apply-log", message=m)
+        r = MessageResult(name='git', message=m)
+        if log:
+            log_xz = lzma.compress(log.encode("utf-8"))
+            log_entry = LogEntry(data_xz=log_xz)
+            log_entry.save()
+            r.log_entry = log_entry
+            if get_property(MessageProperty, "git.apply-failed", message=m):
+                r.status = api.models.Result.FAILURE
+            else:
+                git_repo = get_property(MessageProperty, "git.repo", message=m)
+                git_tag = get_property(MessageProperty, "git.tag", message=m)
+                git_url = get_property(MessageProperty, "git.url", message=m)
+                git_base = get_property(MessageProperty, "git.base", message=m)
+                data = {}
+                if git_repo and git_tag:
+                    data['repo'] = git_repo
+                    data['tag'] = 'refs/tags/' + git_tag
+                    if git_url:
+                        data['url'] = git_url
+                    if git_base:
+                        data['base'] = git_base
+                r.data = data
+                r.status = api.models.Result.SUCCESS
+        else:
+            status = api.models.Result.PENDING
+        r.last_update = datetime.datetime.utcnow()
+        r.save()
+    messages = Message.objects.filter(properties__name='git.apply-log', properties__blob=True)
+    for m in messages:
+        delete_property_blob(MessageProperty, "git.apply-log", message=m)
+    MessageProperty.objects.filter(name__startswith='git.').delete()
+
+def result_to_properties(apps, schema_editor):
+    # We can't import the models directly as they may be a newer
+    # version than this migration expects. We use the historical version.
+    Message = apps.get_model('api', 'Message')
+    MessageProperty = apps.get_model('api', 'MessageProperty')
+    MessageResult = apps.get_model('api', 'MessageResult')
+    LogEntry = apps.get_model('api', 'LogEntry')
+    messages = Message.objects.filter(results__name='git')
+    for m in messages:
+        r = MessageResult.objects.get(name='git', message=m)
+        if not r:
+            continue
+        if r.status == api.models.Result.PENDING:
+            set_property(MessageProperty, 'git.need-apply', True, message=m)
+        else:
+            log = lzma.decompress(r.log_entry.data_xz).decode("utf-8")
+            set_property(MessageProperty, 'git.need-apply', False, message=m)
+            set_property(MessageProperty, 'git.apply-log', log, message=m)
+            if r.status == api.models.Result.FAILURE:
+                set_property(MessageProperty, "git.apply-failed", True, message=m)
+            else:
+                set_property(MessageProperty, "git.apply-failed", False, message=m)
+                if 'repo' in r.data:
+                    set_property(MessageProperty, "git.repo", r.data['repo'], message=m)
+                if 'tag' in r.data:
+                    set_property(MessageProperty, "git.tag", r.data['repo'][len('refs/tags/'):], message=m)
+                if 'url' in r.data:
+                    set_property(MessageProperty, "git.url", r.data['url'], message=m)
+                if 'base' in r.data:
+                    set_property(MessageProperty, "git.base", r.data['base'], message=m)
+    MessageResult.objects.filter(message=m, name='git').delete()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0027_auto_20180521_0152'),
+    ]
+
+    operations = [
+        migrations.RunPython(result_from_properties,
+                             reverse_code=result_to_properties),
+    ]
diff --git a/api/migrations/__init__.py b/api/migrations/__init__.py
index e69de29..f4b7eb7 100644
--- a/api/migrations/__init__.py
+++ b/api/migrations/__init__.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+#
+# Copyright 2018 Red Hat, Inc.
+#
+# Authors:
+#     Paolo Bonzini <pbonzini at redhat.com>
+#
+# This work is licensed under the MIT License.  Please see the LICENSE file or
+# http://opensource.org/licenses/MIT.
+
+
+import json
+from api import blobs
+
+def load_blob_json_safe(name):
+    try:
+        return json.loads(blobs.load_blob(name))
+    except Exception as e:
+        return 'Failed to load blob %s: %s' % (name, e)
+
+def get_property_raw(model, name, **kwargs):
+    try:
+        mp = model.objects.get(name=name, **kwargs)
+        return mp
+    except model.DoesNotExist:
+        return None
+
+def load_property(mp):
+    if mp.blob:
+        return load_blob_json_safe(mp.value)
+    else:
+        return json.loads(mp.value)
+
+def get_property(model, name, **kwargs):
+    mp = get_property_raw(model, name, **kwargs)
+    return load_property(mp) if mp else None
+
+def delete_property_blob(model, name, **kwargs):
+    mp = get_property_raw(model, name, **kwargs)
+    if mp.blob:
+        blobs.delete_blob(mp.value)
+
+def set_property(model, name, value, **kwargs):
+    if value is not None:
+        value = json.dumps(value)
+    mp, created = model.objects.get_or_create(name=name, **kwargs)
+    mp.value = value
+    mp.blob = False
+    mp.save()
diff --git a/mods/git.py b/mods/git.py
index fadce4c..b8c2ff0 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -18,7 +18,7 @@ from django.core.exceptions import PermissionDenied
 from django.utils.html import format_html
 from mod import PatchewModule
 from event import declare_event, register_handler, emit_event
-from api.models import Message, MessageProperty, Result, ResultTuple
+from api.models import Message, MessageProperty, Project, Result, ResultTuple
 from api.rest import PluginMethodField
 from api.views import APILoginRequiredView, prepare_series
 from patchew.logviewer import LogView
@@ -26,16 +26,24 @@ from schema import *
 
 _instance = None
 
+def _get_git_result(msg):
+    try:
+        return msg.results.get(name="git")
+    except:
+        return None
+Message.git_result = property(_get_git_result)
+
+
 class GitLogViewer(LogView):
     def content(self, request, **kwargs):
         series = kwargs['series']
         obj = Message.objects.find_series(series)
         if not obj:
             raise Http404("Object not found: " + series)
-        log = obj.get_property("git.apply-log")
-        if log is None:
+        r = obj.git_result
+        if r is None or not r.is_completed():
             raise Http404("Git apply log not found")
-        return log
+        return r.log
 
 
 class GitModule(PatchewModule):
@@ -66,9 +74,16 @@ class GitModule(PatchewModule):
         register_handler("SeriesComplete", self.on_series_update)
         register_handler("TagsUpdate", self.on_series_update)
 
+    def mark_as_pending_apply(self, series):
+        r = series.git_result or series.create_result(name='git')
+        r.log = None
+        r.status = Result.PENDING
+        r.data = {}
+        r.save()
+
     def on_series_update(self, event, series, **params):
         if series.is_complete:
-            series.set_property("git.need-apply", True)
+            self.mark_as_pending_apply(series)
 
     def get_project_config(self, project, what):
         return project.get_property("git." + what)
@@ -114,50 +129,20 @@ class GitModule(PatchewModule):
 
     def get_based_on(self, message, request, format):
         git_base = self.get_base(message)
-        if not git_base:
-            return None
-
-        return {
-             "repo": git_base.get_property("git.repo"),
-             "tag": 'refs/tags/' + git_base.get_property("git.tag")
-        }
-
+        return git_base.data if git_base else None
 
     def rest_series_fields_hook(self, request, fields, detailed):
         fields['based_on'] = PluginMethodField(obj=self, required=False)
 
     def rest_results_hook(self, obj, results, detailed=False):
-        if not isinstance(obj, Message):
-            return
-        log = obj.get_property("git.apply-log")
-        data = None
-        if log:
-            if obj.get_property("git.apply-failed"):
-                status = Result.FAILURE
-            else:
-                git_repo = obj.get_property("git.repo")
-                git_tag = obj.get_property("git.tag")
-                git_url = obj.get_property("git.url")
-                git_base = obj.get_property("git.base")
-                data = {}
-                if git_repo and git_tag:
-                    data['repo'] = git_repo
-                    data['tag'] = 'refs/tags/' + git_tag
-                    if git_url:
-                        data['url'] = git_url
-                    if git_base:
-                        data['base'] = git_base
-                status = Result.SUCCESS
-        else:
-            status = Result.PENDING
-        results.append(ResultTuple(name='git', obj=obj, status=status,
-                              log=log, data=data, renderer=self))
+        Result.get_result_tuples(obj, "git", results)
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
             return
-        if message.get_property("git.apply-log"):
-            if message.get_property("git.apply-failed"):
+        r = message.git_result
+        if r and r.is_completed():
+            if r.is_failure():
                 title = "Failed in applying to current master"
                 message.status_tags.append({
                     "title": title,
@@ -165,10 +150,10 @@ class GitModule(PatchewModule):
                     "char": "G",
                     })
             else:
-                git_url = message.get_property("git.url")
-                git_repo = message.get_property("git.repo")
-                git_tag = message.get_property("git.tag")
+                git_url = r.data.get('url')
                 if git_url:
+                    git_repo = r.data['repo']
+                    git_tag = r.data['tag']
                     message.status_tags.append({
                         "url": git_url,
                         "title": format_html("Applied as tag {} in repo {}", git_tag, git_repo),
@@ -181,9 +166,7 @@ class GitModule(PatchewModule):
                         "type": "info",
                         "char": "G",
                         })
-        if request.user.is_authenticated:
-            if message.get_property("git.apply-failed") != None or \
-                 message.get_property("git.need-apply") == None:
+            if request.user.is_authenticated:
                 url = reverse("git_reset",
                               kwargs={"series": message.message_id})
                 message.extra_ops.append({"url": url,
@@ -238,7 +221,8 @@ class GitModule(PatchewModule):
                     filter(project=series.project, message_id=base_id).first()
             if not base:
                 return None
-            return base if base.get_property("git.repo") else None
+            r = base.git_result
+            return r if r and r.data.get("repo") else None
 
     def prepare_series_hook(self, request, series, response):
         po = series.project
@@ -247,8 +231,8 @@ class GitModule(PatchewModule):
                 response[prop] = po.get_property(prop)
         base = self.get_base(series)
         if base:
-            response["git.repo"] = base.get_property("git.repo")
-            response["git.base"] = base.get_property("git.tag")
+            response["git.repo"] = base.data["repo"]
+            response["git.base"] = base.data["tag"]
 
     def _poll_project(self, po):
         repo, branch = self._get_project_repo_and_branch(po)
@@ -268,10 +252,7 @@ class GitModule(PatchewModule):
         obj = Message.objects.find_series(series)
         if not obj:
             raise Http404("Not found: " + series)
-        for p in obj.get_properties():
-            if p.startswith("git.") and p != "git.need-apply":
-                obj.set_property(p, None)
-        obj.set_property("git.need-apply", True)
+        self.mark_as_pending_apply(obj)
         return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
 
     def www_url_hook(self, urlpatterns):
@@ -287,11 +268,9 @@ class ApplierGetView(APILoginRequiredView):
     allowed_groups = ["importers"]
 
     def handle(self, request):
-        mp = MessageProperty.objects.filter(name="git.need-apply",
-                                            value='true',
-                                            message__is_complete=True).first()
-        if mp:
-            return prepare_series(request, mp.message)
+        m = Message.objects.filter(results__name="git", results__status="pending").first()
+        if m:
+            return prepare_series(request, m)
 
 class ApplierReportView(APILoginRequiredView):
     name = "applier-report"
@@ -299,12 +278,23 @@ class ApplierReportView(APILoginRequiredView):
 
     def handle(self, request, project, message_id, tag, url, base, repo,
                failed, log):
-        s = Message.objects.series_heads().get(project__name=project,
-                                               message_id=message_id)
-        s.set_property("git.tag", tag)
-        s.set_property("git.url", url)
-        s.set_property("git.base", base)
-        s.set_property("git.repo", repo)
-        s.set_property("git.apply-failed", failed)
-        s.set_property("git.apply-log", log)
-        s.set_property("git.need-apply", False)
+        p = Project.objects.get(name=project)
+        r = Message.objects.series_heads().get(project=p,
+                                               message_id=message_id).git_result
+        r.log = log
+        if failed:
+            r.status = Result.FAILURE
+        else:
+            data = {}
+            data['repo'] = repo
+            data['tag'] = 'refs/tags/' + tag
+            if url:
+                data['url'] = url
+            elif url_template and tag:
+                url_template = p.get_property("git.url_template")
+                data['url'] = url_template.replace("%t", tag)
+            if base:
+                data['base'] = base
+            r.data = data
+            r.status = Result.SUCCESS
+        r.save()
diff --git a/tests/test_git.py b/tests/test_git.py
index d64f901..509949a 100755
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -14,7 +14,7 @@ sys.path.append(os.path.dirname(__file__))
 from patchewtest import PatchewTestCase, main
 import shutil
 import subprocess
-from api.models import Message
+from api.models import Message, Result
 
 class GitTest(PatchewTestCase):
 
@@ -34,37 +34,37 @@ class GitTest(PatchewTestCase):
     def do_apply(self):
         self.cli(["apply", "--applier-mode"])
         for s in Message.objects.series_heads():
-            self.assertFalse(s.get_property("git.need-apply"))
+            self.assertNotEqual(s.git_result.status, Result.PENDING)
 
     def test_need_apply(self):
         self.cli_import("0001-simple-patch.mbox.gz")
         s = Message.objects.series_heads()[0]
         self.assertEqual(s.is_complete, True)
-        self.assertEqual(s.get_property("git.need-apply"), True)
+        self.assertEqual(s.git_result.status, Result.PENDING)
         self.do_apply()
 
     def test_need_apply_multiple(self):
         self.cli_import("0004-multiple-patch-reviewed.mbox.gz")
         s = Message.objects.series_heads()[0]
         self.assertEqual(s.is_complete, True)
-        self.assertEqual(s.get_property("git.need-apply"), True)
+        self.assertEqual(s.git_result.status, Result.PENDING)
         self.do_apply()
 
     def test_need_apply_incomplete(self):
         self.cli_import("0012-incomplete-series.mbox.gz")
         s = Message.objects.series_heads()[0]
         self.assertEqual(s.is_complete, False)
-        self.assertEqual(s.get_property("git.need-apply"), None)
+        self.assertEqual(s.git_result is None, True)
 
     def test_apply(self):
         self.cli_import("0013-foo-patch.mbox.gz")
         self.do_apply()
         s = Message.objects.series_heads()[0]
         self.assertEqual(s.is_complete, True)
-        self.assertEqual(s.get_property("git.repo"), self.repo)
-        self.assertEqual(s.get_property("git.tag"),
-                         "patchew/20160628014747.20971-1-famz at redhat.com")
-        self.assertEqual(s.get_property("git.url"),
+        self.assertEqual(s.git_result.data['repo'], self.repo)
+        self.assertEqual(s.git_result.data['tag'],
+                         "refs/tags/patchew/20160628014747.20971-1-famz at redhat.com")
+        self.assertEqual(s.git_result.data['url'],
                          self.repo + " patchew/20160628014747.20971-1-famz at redhat.com")
 
     def test_apply_with_base(self):
@@ -74,10 +74,10 @@ class GitTest(PatchewTestCase):
         self.do_apply()
         s = Message.objects.series_heads().filter(message_id="20160628014747.20971-2-famz at redhat.com")[0]
         self.assertEqual(s.is_complete, True)
-        self.assertEqual(s.get_property("git.repo"), self.repo)
-        self.assertEqual(s.get_property("git.tag"),
-                         "patchew/20160628014747.20971-2-famz at redhat.com")
-        self.assertEqual(s.get_property("git.url"),
+        self.assertEqual(s.git_result.data['repo'], self.repo)
+        self.assertEqual(s.git_result.data['tag'],
+                         "refs/tags/patchew/20160628014747.20971-2-famz at redhat.com")
+        self.assertEqual(s.git_result.data['url'],
                          self.repo + " patchew/20160628014747.20971-2-famz at redhat.com")
 
     def test_apply_with_base_and_brackets(self):
@@ -87,10 +87,10 @@ class GitTest(PatchewTestCase):
         self.do_apply()
         s = Message.objects.series_heads().filter(message_id="20160628014747.20971-2-famz at redhat.com")[0]
         self.assertEqual(s.is_complete, True)
-        self.assertEqual(s.get_property("git.repo"), self.repo)
-        self.assertEqual(s.get_property("git.tag"),
-                         "patchew/20160628014747.20971-2-famz at redhat.com")
-        self.assertEqual(s.get_property("git.url"),
+        self.assertEqual(s.git_result.data['repo'], self.repo)
+        self.assertEqual(s.git_result.data['tag'],
+                         "refs/tags/patchew/20160628014747.20971-2-famz at redhat.com")
+        self.assertEqual(s.git_result.data['url'],
                          self.repo + " patchew/20160628014747.20971-2-famz at redhat.com")
 
     def test_rest_need_apply(self):
-- 
2.17.0





More information about the Patchew-devel mailing list