Bug#756752: tracker.debian.org: Access package screenshots

[Raphael Hertzog <hertzog@debian.org>]

On Mon, 25 Aug 2014, Andrew Starr-Bochicchio wrote:
> The attached patch adds links to screenshots.debian.net when they
> exist. It's slower that I'd like.

Some ideas to improve it:
1/ use a transaction (protect most of execute in "with
transaction.atomic():")
2/ use bulk insert in Django (and bulk delete first in the transaction)
https://docs.djangoproject.com/en/1.6/ref/models/querysets/#django.db.models.query.QuerySet.bulk_create

See UpdatePackageBugStats.update_source_and_pseudo_bugs() for an example.

Performance is rather important. We're looking to have very regular runs
of most tasks.

> +class ScreenshotsLinkTest(TestCase):

> +    def test_source_package_with_screenshot(self):
> +        package_name = SourcePackageName.objects.create(name='dummy')
> +        package = SourcePackage.objects.create(
> +            source_package_name=package_name,
> +            version='1.0.0')

Please factorize this duplicated code in setUp() or some other method.

> +        PackageExtractedInfo.objects.create(
> +            package=package.source_package_name,
> +            key='screenshots',
> +            value={'screenshots': 'true'}
> +        )

That could also be factorized in the same method with the value
given as parameter.

> +@mock.patch('distro_tracker.core.utils.http.requests')
> +class UpdatePackageScreenshotsTaskTest(TestCase):
> +    """
> +    Tests for the :class:`distro_tracker.vendor.debian.tracker_tasks.UpdatePackageScreenshotsTask` task.
> +    """

Please wrap to not go over 80 chars.

Apart from those points, your patch looks good. Can you update your
patch?

TIA.
-- 
Raphaël Hertzog ◈ Debian Developer

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