Merge lp:~doanac/qa-dashboard/smoke-image-unique into lp:qa-dashboard

[Andy Doan]

dashboard api: remove race condition

The dashboard itself is changing to prevent the creation of duplicate
SmokeImage objects. We'll now get this as an http exception when trying
to create an object. We need to handle that by then doing a GET call
to find the SmokeImage created by the other thread that won the race.

This is the backend change needed by:

  https://code.launchpad.net/~doanac/ubuntu-test-cases/ttouch-race/+merge/218335

See commit-message for dirty details

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:741
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/324/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/324/rebuild

review: Approve (continuous-integration)

[Paul Larson (pwlars)]

Looks sane to me, but I'll let someone with more familiarity in the dashboard have the final say.

review: Approve

[Joe Talbott (joetalbott)]

L28 - Doesn't this code get called when there is an existing entry as well? I haven't read the context but it looks like this is in the save() override and my assumption is that save on an existing object instance would raise the DBError here.

review: Needs Information

[lp:~doanac/qa-dashboard/smoke-image-unique]

742. By Andy Doan on 2014-05-06

code-review catch by josepht

The save method was only working if you changed the object in a
certain way. This fixes the check

[Andy Doan (doanac)]

On 05/06/2014 06:28 AM, Joe Talbott wrote:
> Review: Needs Information
>
> L28 - Doesn't this code get called when there is an existing entry as well? I haven't read the context but it looks like this is in the save() override and my assumption is that save on an existing object instance would raise the DBError here.
>
Good catch. In my testing I was always changing something like the
"variant". So it became a new SmokeImage object. However, the code
didn't work if you changed a flag like "publish". This fixes things.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:742
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/325/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/325/rebuild

review: Approve (continuous-integration)

[Joe Talbott (joetalbott)]

This looks okay to me.

review: Approve

=== modified file 'smokeng/models.py'
--- smokeng/models.py	2014-02-18 15:31:31 +0000
+++ smokeng/models.py	2014-05-06 16:26:55 +0000
@@ -18,7 +18,7 @@
 
 from collections import Counter
 
-from django.db import models
+from django.db import models, DatabaseError
 
 from common.models import (
     DashboardBaseModel,
@@ -52,6 +52,19 @@
         '''ensure we have build_number parts'''
         if not self.build_id:
             split_build_number(self, False)
+
+        # make sure we don't have the same image. It would have made more
+        # to have proper primary-keys for this table, but it would make for
+        # weird migration. This is safest and easiest to test for
+        qs = SmokeImage.objects.filter(
+            build_number=self.build_number,
+            release=self.release,
+            flavor=self.flavor,
+            arch=self.arch,
+            variant=self.variant,
+        )
+        if qs.count() != 0 and qs[0].id != self.id:
+            raise DatabaseError('Duplicate image exists')
         return super(SmokeImage, self).save(**kwargs)
 
     def __unicode__(self):

=== modified file 'smokeng/tests.py'
--- smokeng/tests.py	2014-02-27 16:08:17 +0000
+++ smokeng/tests.py	2014-05-06 16:26:55 +0000
@@ -24,6 +24,7 @@
 smokeng.api  # just to silence pep8 warnings. this does nothing
 
 from django.core.urlresolvers import reverse
+from django.db import DatabaseError
 from django.test import TestCase
 from django.test.client import Client
 from django.contrib.auth.models import (
@@ -1010,6 +1011,19 @@
         self.assertEqual(params['release'], obj['release'])
         self.assertEqual(params['build_number'], obj['rootfs_id'])
 
+    def testImageAddDuplicate(self):
+        '''ensure we don't allow duplicate images to be created.'''
+        params = {
+            'build_number': 'build_number',
+            'release': 'release',
+            'flavor': 'flavor',
+            'variant': 'variant',
+            'arch': 'arch',
+        }
+        self._post('/smokeng/api/v1/image/', params)
+        with self.assertRaises(DatabaseError):
+            self._post('/smokeng/api/v1/image/', params)
+
     def testResultAdd(self):
         params = {
             'ran_at': datetime.datetime.now().isoformat(),