[Patchew-devel] [PATCH 06/12] models: create Result model

Paolo Bonzini posted 12 patches 6 years, 11 months ago
[Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Paolo Bonzini 6 years, 11 months ago
The Result model has more or less a 1:1 correspondence with the fields in
the REST results detail view, and thus with ResultSerializerFull.
Until all plugins are converted, everything will still go through
namedtuple results, so the namedtuple is kept, renamed to ResultTuple.
The duplicated code will disappear at the end of this series.

There is an extra field tracking the time of the last update, which will
be used by the testing module; for simplicity it is not yet presented
by the serializers, though it will be added when ResultTuple is dropped.

The external interface of Result and ResultTuple is more or less the same,
except that we have to get rid of the two Python objects that the tuple
stores, obj and renderer.  The renderer is eliminated by convention: the
renderer of the new Result objects is always a PatchewModule, and the name
of the result up to the first period (if any) identifies the renderer.
As for obj, the parent object, the Result object has no clue whether it
comes from a project or a message, so it is passed as a new parameter
to render() and get_log_url().  The new parameter is easy to integrate
in both the REST API SerializerMethodField getter and in the web view.

Compared to properties, logs are always stored in the database, independent
of the size.  However, they are always stored in xz format compared to
the JSON string format used by properties.  Using xz will complicate
migrations a little, since the log property won't be available there,
but not as much as handling blobs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 api/migrations/0027_auto_20180521_0152.py |  38 ++++++++
 api/models.py                             | 103 +++++++++++++++++++---
 mods/git.py                               |   4 +-
 mods/testing.py                           |   6 +-
 4 files changed, 134 insertions(+), 17 deletions(-)
 create mode 100644 api/migrations/0027_auto_20180521_0152.py

diff --git a/api/migrations/0027_auto_20180521_0152.py b/api/migrations/0027_auto_20180521_0152.py
new file mode 100644
index 0000000..94aa0d8
--- /dev/null
+++ b/api/migrations/0027_auto_20180521_0152.py
@@ -0,0 +1,38 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.12 on 2018-05-21 20:05
+from __future__ import unicode_literals
+
+import django.core.validators
+from django.db import migrations, models
+import jsonfield.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0026_auto_20180426_0829'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Result',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('name', models.CharField(max_length=256)),
+                ('last_update', models.DateTimeField()),
+                ('status', models.CharField(db_index=True, max_length=7, validators=[django.core.validators.RegexValidator(code='invalid', message='status must be one of pending, success, failure, running', regex='pending|success|failure|running')])),
+                ('log_xz', models.BinaryField(blank=True, null=True)),
+                ('data', jsonfield.fields.JSONField()),
+            ],
+        ),
+        migrations.AddField(
+            model_name='message',
+            name='results',
+            field=models.ManyToManyField(blank=True, to='api.Result'),
+        ),
+        migrations.AddField(
+            model_name='project',
+            name='results',
+            field=models.ManyToManyField(blank=True, to='api.Result'),
+        ),
+    ]
diff --git a/api/models.py b/api/models.py
index 497e43c..ee54d04 100644
--- a/api/models.py
+++ b/api/models.py
@@ -14,7 +14,9 @@ import json
 import datetime
 import re
 
+from django.core import validators
 from django.db import models
+from django.db.models import Q
 from django.contrib.auth.models import User
 from django.urls import reverse
 import jsonfield
@@ -25,6 +27,86 @@ from event import emit_event, declare_event
 from .blobs import save_blob, load_blob, load_blob_json
 import mod
 
+class Result(models.Model):
+    PENDING = 'pending'
+    SUCCESS = 'success'
+    FAILURE = 'failure'
+    RUNNING = 'running'
+    VALID_STATUSES = (PENDING, SUCCESS, FAILURE, RUNNING)
+    VALID_STATUSES_RE = '|'.join(VALID_STATUSES)
+
+    name = models.CharField(max_length=256)
+    last_update = models.DateTimeField()
+    status = models.CharField(max_length=7, db_index=True, validators=[
+        validators.RegexValidator(regex=VALID_STATUSES_RE,
+                                  message='status must be one of ' + ', '.join(VALID_STATUSES),
+                                  code='invalid')])
+    log_xz = models.BinaryField(blank=True, null=True)
+    data = jsonfield.JSONField()
+
+    def is_success(self):
+        return self.status == self.SUCCESS
+
+    def is_failure(self):
+        return self.status == self.FAILURE
+
+    def is_completed(self):
+        return self.is_success() or self.is_failure()
+
+    def is_pending(self):
+        return self.status == self.PENDING
+
+    def is_running(self):
+        return self.status == self.RUNNING
+
+    def save(self):
+        self.last_update = datetime.datetime.utcnow()
+        return super(Result, self).save()
+
+    @property
+    def renderer(self):
+        found = re.match("^[^.]*", self.name)
+        return mod.get_module(found[0]) if found else None
+
+    def render(self, obj):
+        if self.renderer is None:
+            return None
+        return self.renderer.render_result(self, obj)
+
+    @property
+    def log(self):
+        if hasattr(self, "_log"):
+            return self._log
+        if self.log_xz is None:
+            self._log = None
+        else:
+            self._log = lzma.decompress(self.log_xz).decode("utf-8")
+        return self._log
+
+    @log.setter
+    def log(self, value):
+        self._log = value
+        if value is None:
+            self.log_xz = None
+        else:
+            self.log_xz = lzma.compress(value.encode("utf-8"))
+
+    def get_log_url(self, obj, request=None):
+        if not self.is_completed() or self.renderer is None:
+            return None
+        log_url = self.renderer.get_result_log_url(obj, self.name)
+        if log_url is not None and request is not None:
+            log_url = request.build_absolute_uri(log_url)
+        return log_url
+
+    @staticmethod
+    def get_result_tuples(obj, module, results):
+        name_filter = Q(name=module) | Q(name__startswith=module + '.')
+        renderer = mod.get_module(module)
+        for r in obj.results.filter(name_filter):
+            results.append(ResultTuple(name=r.name, obj=obj, status=r.status,
+                                       log=r.log, data=r.data, renderer=renderer))
+
 class Project(models.Model):
     name = models.CharField(max_length=1024, db_index=True, unique=True,
                             help_text="""The name of the project""")
@@ -58,6 +140,7 @@ class Project(models.Model):
                                        top project which has
                                        parent_project=NULL""")
     maintainers = models.ManyToManyField(User, blank=True)
+    results = models.ManyToManyField(Result, blank=True)
 
     def __str__(self):
         return self.name
@@ -319,6 +402,7 @@ class Message(models.Model):
     num_patches = models.IntegerField(null=False, default=-1, blank=True)
 
     objects = MessageManager()
+    results = models.ManyToManyField(Result, blank=True)
 
     def save_mbox(self, mbox_blob):
         save_blob(mbox_blob, self.message_id)
@@ -595,34 +679,29 @@ class Module(models.Model):
     def __str__(self):
         return self.name
 
-class Result(namedtuple("Result", "name status log obj data renderer")):
+class ResultTuple(namedtuple("ResultTuple", "name status log obj data renderer")):
     __slots__ = ()
-    PENDING = 'pending'
-    SUCCESS = 'success'
-    FAILURE = 'failure'
-    RUNNING = 'running'
-    VALID_STATUSES = (PENDING, SUCCESS, FAILURE, RUNNING)
 
     def __new__(cls, name, status, obj, log=None, data=None, renderer=None):
-        if status not in cls.VALID_STATUSES:
+        if status not in Result.VALID_STATUSES:
             raise ValueError("invalid value '%s' for status field" % status)
-        return super(cls, Result).__new__(cls, status=status, log=log,
+        return super(cls, ResultTuple).__new__(cls, status=status, log=log,
                                           obj=obj, data=data, name=name, renderer=renderer)
 
     def is_success(self):
-        return self.status == self.SUCCESS
+        return self.status == Result.SUCCESS
 
     def is_failure(self):
-        return self.status == self.FAILURE
+        return self.status == Result.FAILURE
 
     def is_completed(self):
         return self.is_success() or self.is_failure()
 
     def is_pending(self):
-        return self.status == self.PENDING
+        return self.status == Result.PENDING
 
     def is_running(self):
-        return self.status == self.RUNNING
+        return self.status == Result.RUNNING
 
     def render(self):
         if self.renderer is None:
diff --git a/mods/git.py b/mods/git.py
index a216ea2..eae9be6 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
+from api.models import Message, MessageProperty, Result, ResultTuple
 from api.rest import PluginMethodField
 from api.views import APILoginRequiredView, prepare_series
 from patchew.logviewer import LogView
@@ -150,7 +150,7 @@ class GitModule(PatchewModule):
                 status = Result.SUCCESS
         else:
             status = Result.PENDING
-        results.append(Result(name='git', obj=obj, status=status,
+        results.append(ResultTuple(name='git', obj=obj, status=status,
                               log=log, data=data, renderer=self))
 
     def prepare_message_hook(self, request, message, detailed):
diff --git a/mods/testing.py b/mods/testing.py
index 2d379cb..98732e3 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -17,7 +17,7 @@ from mod import PatchewModule
 import time
 import math
 from api.views import APILoginRequiredView
-from api.models import Message, MessageProperty, Project, Result
+from api.models import Message, MessageProperty, Project, Result, ResultTuple
 from api.search import SearchEngine
 from event import emit_event, declare_event, register_handler
 from patchew.logviewer import LogView
@@ -270,12 +270,12 @@ class TestingModule(PatchewModule):
 
             data = p.copy()
             del data['passed']
-            results.append(Result(name='testing.' + tn, obj=obj, status=passed_str,
+            results.append(ResultTuple(name='testing.' + tn, obj=obj, status=passed_str,
                                   log=log, data=data, renderer=self))
 
         if obj.get_property("testing.ready"):
             for tn in all_tests:
-                results.append(Result(name='testing.' + tn, obj=obj, status='pending'))
+                results.append(ResultTuple(name='testing.' + tn, obj=obj, status='pending'))
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
-- 
2.17.0


_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Fam Zheng 6 years, 11 months ago
On Tue, 05/22 08:57, Paolo Bonzini wrote:
> @@ -58,6 +140,7 @@ class Project(models.Model):
>                                         top project which has
>                                         parent_project=NULL""")
>      maintainers = models.ManyToManyField(User, blank=True)
> +    results = models.ManyToManyField(Result, blank=True)
>  
>      def __str__(self):
>          return self.name
> @@ -319,6 +402,7 @@ class Message(models.Model):
>      num_patches = models.IntegerField(null=False, default=-1, blank=True)
>  
>      objects = MessageManager()
> +    results = models.ManyToManyField(Result, blank=True)

Why ManyToMany relationship? A Result entry belongs to exactly one Project or
Message, just like Property, no?

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Paolo Bonzini 6 years, 11 months ago
On 22/05/2018 10:11, Fam Zheng wrote:
> On Tue, 05/22 08:57, Paolo Bonzini wrote:
>> @@ -58,6 +140,7 @@ class Project(models.Model):
>>                                         top project which has
>>                                         parent_project=NULL""")
>>      maintainers = models.ManyToManyField(User, blank=True)
>> +    results = models.ManyToManyField(Result, blank=True)
>>  
>>      def __str__(self):
>>          return self.name
>> @@ -319,6 +402,7 @@ class Message(models.Model):
>>      num_patches = models.IntegerField(null=False, default=-1, blank=True)
>>  
>>      objects = MessageManager()
>> +    results = models.ManyToManyField(Result, blank=True)
> Why ManyToMany relationship? A Result entry belongs to exactly one Project or
> Message, just like Property, no?

Yes, however I wanted to avoid the same issue like we have with
properties, where we need two classes ProjectProperty or MessageProperty
for basically the same model.  And if I add a OneToManyField, I'll have
two database fields in the api_result table (for the project_id and the
message_id).

It's not great but I don't have a better idea, it's the best I could do
(start) on a plane without StackOverflow. :)  That's also why the series
is RFC.

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Fam Zheng 6 years, 11 months ago
On Tue, 05/22 16:57, Paolo Bonzini wrote:
> On 22/05/2018 10:11, Fam Zheng wrote:
> > On Tue, 05/22 08:57, Paolo Bonzini wrote:
> >> @@ -58,6 +140,7 @@ class Project(models.Model):
> >>                                         top project which has
> >>                                         parent_project=NULL""")
> >>      maintainers = models.ManyToManyField(User, blank=True)
> >> +    results = models.ManyToManyField(Result, blank=True)
> >>  
> >>      def __str__(self):
> >>          return self.name
> >> @@ -319,6 +402,7 @@ class Message(models.Model):
> >>      num_patches = models.IntegerField(null=False, default=-1, blank=True)
> >>  
> >>      objects = MessageManager()
> >> +    results = models.ManyToManyField(Result, blank=True)
> > Why ManyToMany relationship? A Result entry belongs to exactly one Project or
> > Message, just like Property, no?
> 
> Yes, however I wanted to avoid the same issue like we have with
> properties, where we need two classes ProjectProperty or MessageProperty
> for basically the same model.  And if I add a OneToManyField, I'll have
> two database fields in the api_result table (for the project_id and the
> message_id).
> 
> It's not great but I don't have a better idea, it's the best I could do
> (start) on a plane without StackOverflow. :)  That's also why the series
> is RFC.

Maybe we could create a dummy Message for each Project as a host of its results,
so we can forward the Project.results property to the Message entry? (The same
applies to ProjectProperty). It's also quite ugly, but perhaps slighly more
straightforward than this.

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Paolo Bonzini 6 years, 11 months ago
On 22/05/2018 17:14, Fam Zheng wrote:
>>
>> It's not great but I don't have a better idea, it's the best I could do
>> (start) on a plane without StackOverflow. :)  That's also why the series
>> is RFC.
> Maybe we could create a dummy Message for each Project as a host of its results,
> so we can forward the Project.results property to the Message entry? (The same
> applies to ProjectProperty). It's also quite ugly, but perhaps slighly more
> straightforward than this.

I don't know.  That seems uglier, and unlike this it would have
consequences outside the results subsystem (what I like about results is
that they sit on top of the rest, if they disappeared nobody would
notice outside the git and testing plugins).

There must be some way to do it in Django, perhaps with a custom manager
or a custom intermediate table on the many-to-many relation that has an
uniqueness constraint on the result_id.  In the end ManyToManyField is
just a wrapper around a custom manager, itself.

So I think we should look at the database structure and decide if it's
okay.  If it is, how we represent it in Django is relatively
unimportant.  Of the two, I somewhat prefer the two intermediate tables
from ManyToManyField, rather than the two fields in the result tables
from OneToManyField.

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Fam Zheng 6 years, 11 months ago
On Tue, 05/22 17:33, Paolo Bonzini wrote:
> On 22/05/2018 17:14, Fam Zheng wrote:
> So I think we should look at the database structure and decide if it's
> okay.  If it is, how we represent it in Django is relatively
> unimportant.  Of the two, I somewhat prefer the two intermediate tables
> from ManyToManyField, rather than the two fields in the result tables
> from OneToManyField.

Maybe this works:

https://godjango.com/blog/django-abstract-base-class-multi-table-inheritance/

?

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Paolo Bonzini 6 years, 11 months ago
On 23/05/2018 03:28, Fam Zheng wrote:
> On Tue, 05/22 17:33, Paolo Bonzini wrote:
>> On 22/05/2018 17:14, Fam Zheng wrote:
>> So I think we should look at the database structure and decide if it's
>> okay.  If it is, how we represent it in Django is relatively
>> unimportant.  Of the two, I somewhat prefer the two intermediate tables
>> from ManyToManyField, rather than the two fields in the result tables
>> from OneToManyField.
> 
> Maybe this works:
> 
> https://godjango.com/blog/django-abstract-base-class-multi-table-inheritance/

Related to this, should the logs be in a separate table with 1-to-1
relation?  This would be less hackish than peppering defer('log_xz')
everywhere.

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH 06/12] models: create Result model
Posted by Paolo Bonzini 6 years, 11 months ago
On 30/05/2018 00:39, Paolo Bonzini wrote:
> On 23/05/2018 03:28, Fam Zheng wrote:
>> On Tue, 05/22 17:33, Paolo Bonzini wrote:
>>> On 22/05/2018 17:14, Fam Zheng wrote:
>>> So I think we should look at the database structure and decide if it's
>>> okay.  If it is, how we represent it in Django is relatively
>>> unimportant.  Of the two, I somewhat prefer the two intermediate tables
>>> from ManyToManyField, rather than the two fields in the result tables
>>> from OneToManyField.
>>
>> Maybe this works:
>>
>> https://godjango.com/blog/django-abstract-base-class-multi-table-inheritance/
> 
> Related to this, should the logs be in a separate table with 1-to-1
> relation?  This would be less hackish than peppering defer('log_xz')
> everywhere.

Oh, and regarding the post itself, that seems great.  It should create
exactly the same tables, but with a better mapping back to objects.
I'll take a look when I have some time.

Thanks,

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel