[PATCH 04/10] models: Add 'status' model

Finucane, Stephen stephen.finucane at intel.com
Thu Sep 24 03:14:13 AEST 2015


> On Wed, Jul 29, 2015 at 09:58:42AM +0100, Stephen Finucane wrote:
> > +class Status(models.Model):
> > +    """Status for a patch.
> > +
> > +    Statuses define a state for patches. This is useful, for example,
> > +    when using a continuous integration (CI) system to test patches.
> > +    """
> > +    STATE_PENDING = 0
> > +    STATE_SUCCESS = 1
> > +    STATE_WARNING = 2
> > +    STATE_FAIL = 3
> > +    STATE_CHOICES = (
> > +        (STATE_PENDING, 'pending'),
> > +        (STATE_SUCCESS, 'success'),
> > +        (STATE_WARNING, 'warning'),
> > +        (STATE_FAIL, 'fail'),
> > +    )
> > +
> > +    patch = models.ForeignKey(Patch)
> > +    user = models.ForeignKey(User)
> > +    date = models.DateTimeField(default=datetime.datetime.now)
> > +
> > +    state = models.SmallIntegerField(
> > +        choices=STATE_CHOICES, default=STATE_PENDING,
> > +        help_text='The state of the status.')
> > +    target_url = models.URLField(
> > +        blank=True, null=True,
> > +        help_text='The target URL to associate with this status. This
> should'
> > +        ' be specific to the patch.')
> > +    description = models.TextField(
> > +        blank=True, null=True, help_text='A brief description of the
> status.')
> > +    context = models.CharField(
> > +        max_length=255, default='default', blank=True, null=True,
> > +        help_text='A label to discern status from statuses of other
> systems.')
> 
> What I was thinking:
> 
>   - For each project, we have a list of Tests defined (because different
>     patchwork projects will have different tests). That also allows us
>     to know when all the tests have run for that specific project, a
>     useful property to send back emails with all the test results to the
>     ml for instance,
>   - For each (Test, Patch) there's a TestResult (which would roughly
>     correspond to your Status, but some of the fields there seem to
>     belong to a Test object,
>   - I'd love if tests could also be at the Series level, because it may
>     be impractical for some tests to be every patch (that's certainly
>     the case for the i915 driver).
>
> class Test(...):
>     name
>     description
>     project_id
>     ...
> 
> class PatchTestResult(...)
>     test_id
>     patch_id
>     result (the list of 'state choices' you had)
>     result_url
>     ...
> 
> Later on we could have:
> 
> class SeriesTestResult(...)
>     test_id
>     series_id
>     result (the list of 'state choices' you had)
>     result_url
>     ...
> 
> Thoughts?

Thanks for the review: appreciate the feedback. The existing proposal was developed after a lot of research into how existing CI integrations work (mostly GitHub and Gerrit). I have also discussed and reviewed this proposal with folks from the DPDK (here on the ML) and Intel Open vSwitch team (privately) and it fits both team's needs. As such, can you explain what advantages the above proposal would introduce over the existing proposal? What does i915 need to do differently? If not, I'd like to rework the patch per existing comments (i.e. renaming 'Status' to 'Services' or 'Test Status') and merge.

One point regarding the "SeriesTestResult": I had planned to implement this as a "lowest common denominator" property. That is, if a patch in a series fail, the series is marked as fail etc. What you describe (a unique, per series property) is quite a limited use case but its behavior could be emulated by only testing the last patch in a series. This would require very little "smarts" on your client end.

> --
> Damien


More information about the Patchwork mailing list