Bug#753910: [PATCH] Add support for showing Debci test failures as an action item. Closes: #753910

[Raphael Hertzog <hertzog@debian.org>]

Hi Andrew, thanks for the patch!

Here are my comments:

On Sun, 06 Jul 2014, Andrew Starr-Bochicchio wrote:
> +@python_2_unicode_compatible
> +class DebciStatus(models.Model):
> +    """
> +    Model for debci status of packages.
> +    """
> +    package = models.OneToOneField(PackageName, related_name='debci_status')
> +    status = JSONField()

Please don't add a new model for this. Rather use a new "key" in
PackageExtractedInfo.

PackageExtractedInfo model should probably be renamed into something else
but it's the better place to store this as we can then query most of the
relevant information for a single package in a single query...

Most of the rest looks fine, except for a detail:

> +    def execute(self):
> +        all_debci_status = self.get_debci_status()
> +        if not all_debci_status:
> +            return
> +
> +        # Discard all old
> +        DebciStatus.objects.all().delete()
> +
> +        failures = []
> +        packages = []
> +        for result in all_debci_status:
> +            if result['status'] == 'fail':
> +                package = PackageName.objects.get(name=result['package'])
> +                debci_status = DebciStatus(package=package,
> +                                           status=result)
> +                failures.append(debci_status)
> +                packages.append(package)
> +                self.update_action_item(package, result)
> +
> +        # Remove action items for packages without failing tests.
> +        ActionItem.objects.delete_obsolete_items(
> +            [self.debci_action_item_type], packages)
> +
> +        DebciStatus.objects.bulk_create(failures)

It would be better to do this in a transaction so that we have no time
where the information is lost. Either we have the new content or the old
content, but never an empty list of issues.

It's easy to do:

with transaction.atomic():
    # here goes the protected content

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Discover the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/