Merge lp:~stylesen/lava-scheduler/multinode into lp:lava-scheduler/multinode

[Senthil Kumaran S]

Check device availability during job submission, irrespective of what mode in which the job gets submitted.

[Stevan Radaković (stevanr)]

Hey Senthil, looking good.

Couple of propositions:

29 + except Exception as e:
30 + raise xmlrpclib.Fault(400, str(e))

I'd really like it better if we had a specialized exception for this one, using generic exception for special kind of problem is not really a good idea (something else goes wrong, or some uncaught exception is thrown, someone may thing there was not enough devices and this wasn't the case). So introducing something like DevicesUnavailableException would be good.

42 +def check_device_availability(json_data):
43 + """Checks whether the number of devices requested in job json is
44 + available.
45 +
46 + Returns True if the requested number of devices are available, else
47 + raises an Exception.
48 + """
49 + requested_devices = utils.requested_device_count(json_data)

Increasing re-usability of methods is always a good idea, so it would be better that this one actually takes the requested_devices as an argument, and actually parse the json_data from the api call (someone who needs this method coming from not api side will not be able to pass json_data etc..)

review: Needs Fixing (code)

[Senthil Kumaran S (stylesen)]

Hi Stevan,

Thanks for the review.

On Friday 19 July 2013 03:01 PM, Stevan Radaković wrote:
> Couple of propositions:
>
> 29 + except Exception as e:
> 30 + raise xmlrpclib.Fault(400, str(e))
>
> I'd really like it better if we had a specialized exception for this one, using generic exception for special kind of problem is not really a good idea (something else goes wrong, or some uncaught exception is thrown, someone may thing there was not enough devices and this wasn't the case). So introducing something like DevicesUnavailableException would be good.

I fixed this.

> 42 +def check_device_availability(json_data):
> 43 + """Checks whether the number of devices requested in job json is
> 44 + available.
> 45 +
> 46 + Returns True if the requested number of devices are available, else
> 47 + raises an Exception.
> 48 + """
> 49 + requested_devices = utils.requested_device_count(json_data)
>
> Increasing re-usability of methods is always a good idea, so it would be better that this one actually takes the requested_devices as an argument, and actually parse the json_data from the api call (someone who needs this method coming from not api side will not be able to pass json_data etc..)

This is fixed.

I request your to review the latest changes in this MP.

Thank You.
--
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/

[Stevan Radaković (stevanr)]

Great work.
Thanks

Approve +1

review: Approve

[lp:~stylesen/lava-scheduler/multinode]

264. By Senthil Kumaran S on 2013-07-22

Merge changes for improved device unavailability warning.

=== modified file 'lava_scheduler_app/api.py'
--- lava_scheduler_app/api.py	2013-07-03 07:49:32 +0000
+++ lava_scheduler_app/api.py	2013-07-22 09:37:29 +0000
@@ -7,6 +7,7 @@
     Device,
     DeviceType,
     JSONDataError,
+    DevicesUnavailableException,
     TestJob,
     )
 from lava_scheduler_app.views import (
@@ -28,20 +29,6 @@
                 "'lava_scheduler_app.add_testjob' permission.  Contact "
                 "the administrators." % self.user.username)
         try:
-            requested_devices = utils.requested_device_count(job_data)
-            if requested_devices:
-                all_devices = {}
-                for d in self.all_device_types():
-                    all_devices[d['name']] = d['idle'] + d['busy']
-
-                for board, count in requested_devices.iteritems():
-                    if all_devices.get(board, None) and \
-                            count <= all_devices[board]:
-                        continue
-                    else:
-                        raise xmlrpclib.Fault(
-                            400, "Required number of device(s) unavailable.")
-
             job = TestJob.from_json_and_user(job_data, self.user)
         except JSONDecodeError as e:
             raise xmlrpclib.Fault(400, "Decoding JSON failed: %s." % e)
@@ -51,6 +38,8 @@
             raise xmlrpclib.Fault(404, "Specified device not found.")
         except DeviceType.DoesNotExist:
             raise xmlrpclib.Fault(404, "Specified device type not found.")
+        except DevicesUnavailableException as e:
+            raise xmlrpclib.Fault(400, str(e))
         if isinstance(job, type(list())):
             return job
         else:

=== modified file 'lava_scheduler_app/models.py'
--- lava_scheduler_app/models.py	2013-07-04 05:22:29 +0000
+++ lava_scheduler_app/models.py	2013-07-22 09:37:29 +0000
@@ -29,6 +29,10 @@
     """Error raised when JSON is syntactically valid but ill-formed."""
 
 
+class DevicesUnavailableException(UserWarning):
+    """Error raised when required number of devices are unavailable."""
+
+
 class Tag(models.Model):
 
     name = models.SlugField(unique=True)
@@ -47,6 +51,38 @@
         raise ValidationError(e)
 
 
+def check_device_availability(requested_devices):
+    """Checks whether the number of devices requested is available.
+    
+    See utils.requested_device_count() for details of REQUESTED_DEVICES
+    dictionary format.
+
+    Returns True if the requested number of devices are available, else
+    raises DevicesUnavailableException.
+    """
+    device_types = DeviceType.objects.values_list('name').filter(
+        models.Q(device__status=Device.IDLE) | \
+            models.Q(device__status=Device.RUNNING)
+        ).annotate(
+        num_count=models.Count('name')
+        ).order_by('name')
+
+    if requested_devices:
+        all_devices = {}
+        for dt in device_types:
+            # dt[0] -> device type name
+            # dt[1] -> device type count
+            all_devices[dt[0]] = dt[1]
+
+        for board, count in requested_devices.iteritems():
+            if all_devices.get(board, None) and count <= all_devices[board]:
+                continue
+            else:
+                raise DevicesUnavailableException(
+                    "Required number of device(s) unavailable.")
+    return True
+
+
 class DeviceType(models.Model):
     """
     A class of device, for example a pandaboard or a snowball.
@@ -407,6 +443,8 @@
 
     @classmethod
     def from_json_and_user(cls, json_data, user, health_check=False):
+        requested_devices = utils.requested_device_count(json_data)
+        check_device_availability(requested_devices)
         job_data = simplejson.loads(json_data)
         validate_job_data(job_data)
         if 'target' in job_data: