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/ -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
