Merge lp:~gz/pyjuju/maas_int_constraints_1061286 into lp:pyjuju

[Martin Packman]

Give integer maas constraints without decimal

Currently generic cpu and mem constraints are cast to float inside
juju, this isn't really useful for memory in MB or cpu count as used
by MaaS. It also happens to upset the current code. Giving the
integer form instead is reasonable.

https://codereview.appspot.com/6601060/

[Martin Packman (gz)]

Reviewers: mp+127987_code.launchpad.net,

Message:
Please take a look.

Description:
Give integer maas constraints without decimal

Currently generic cpu and mem constraints are cast to float inside
juju, this isn't really useful for memory in MB or cpu count as used
by MaaS. It also happens to upset the current code. Giving the
integer form instead is reasonable.

https://code.launchpad.net/~gz/juju/maas_int_constraints_1061286/+merge/127987

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6601060/

Affected files:
   A [revision details]
   M juju/providers/maas/maas.py
   M juju/providers/maas/tests/test_maas.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: juju/providers/maas/maas.py
=== modified file 'juju/providers/maas/maas.py'
--- juju/providers/maas/maas.py 2012-09-28 11:50:35 +0000
+++ juju/providers/maas/maas.py 2012-10-04 11:16:58 +0000
@@ -42,14 +42,19 @@
          return match.group("system_id")

+def _int_str(value):
+ """Convert value to integer then string"""
+ return str(int(value))
+
+
  class MAASClient(MAASOAuthConnection):

      _handled_constraints = (
- ("maas-name", "name"),
- ("maas-tags", "tags"),
- ("arch", "arch"),
- ("cpu", "cpu_count"),
- ("mem", "mem"),
+ ("maas-name", "name", str),
+ ("maas-tags", "tags", str),
+ ("arch", "arch", str),
+ ("cpu", "cpu_count", _int_str),
+ ("mem", "mem", _int_str),
          )

      def __init__(self, config):
@@ -127,10 +132,10 @@
          """
          params = {"op": "acquire"}
          if constraints is not None:
- for key_from, key_to in self._handled_constraints:
+ for key_from, key_to, translate in self._handled_constraints:
                  value = constraints.get(key_from, None)
                  if value is not None:
- params[key_to] = str(value)
+ params[key_to] = translate(value)
          return self.post("api/1.0/nodes/", params)

      def start_node(self, resource_uri, ubuntu_series, user_data):

Index: juju/providers/maas/tests/test_maas.py
=== modified file 'juju/providers/maas/tests/test_maas.py'
--- juju/providers/maas/tests/test_maas.py 2012-09-28 11:50:35 +0000
+++ juju/providers/maas/tests/test_maas.py 2012-10-04 11:16:58 +0000
@@ -348,7 +348,7 @@

      def test_acquire_node_handles_cpu_constraint(self):
          client = self.set_up_client_with_fake()
- constraints = {"cpu": 2}
+ constraints = {"cpu": 2.0}
          client.acquire_node(constraints)

          cpu_count = client.post.params_used.get("cpu_count")
@@ -356,7 +356,7 @@

      def test_acquire_node_handles_mem_constraint(self):
          client = self.set_up_client_with_fake()
- constraints = {"mem": 2048}
+ constraints = {"mem": 2048.0}
          client.acquire_node(constraints)

          mem =...

[Kapil Thangavelu (hazmat)]

lgtm, fwiw, the float usage is based on some of the cloud providers
using them for instance type sizing for cpu. the conversion at
usage/provider interaction time here looks good.

https://codereview.appspot.com/6601060/

=== modified file 'juju/providers/maas/maas.py'
--- juju/providers/maas/maas.py	2012-09-28 11:50:35 +0000
+++ juju/providers/maas/maas.py	2012-10-04 11:41:23 +0000
@@ -42,14 +42,19 @@
         return match.group("system_id")
 
 
+def _int_str(value):
+    """Convert value to integer then string"""
+    return str(int(value))
+
+
 class MAASClient(MAASOAuthConnection):
 
     _handled_constraints = (
-        ("maas-name", "name"),
-        ("maas-tags", "tags"),
-        ("arch", "arch"),
-        ("cpu", "cpu_count"),
-        ("mem", "mem"),
+        ("maas-name", "name", str),
+        ("maas-tags", "tags", str),
+        ("arch", "arch", str),
+        ("cpu", "cpu_count", _int_str),
+        ("mem", "mem", _int_str),
         )
 
     def __init__(self, config):
@@ -127,10 +132,10 @@
         """
         params = {"op": "acquire"}
         if constraints is not None:
-            for key_from, key_to in self._handled_constraints:
+            for key_from, key_to, translate in self._handled_constraints:
                 value = constraints.get(key_from, None)
                 if value is not None:
-                    params[key_to] = str(value)
+                    params[key_to] = translate(value)
         return self.post("api/1.0/nodes/", params)
 
     def start_node(self, resource_uri, ubuntu_series, user_data):

=== modified file 'juju/providers/maas/tests/test_maas.py'
--- juju/providers/maas/tests/test_maas.py	2012-09-28 11:50:35 +0000
+++ juju/providers/maas/tests/test_maas.py	2012-10-04 11:41:23 +0000
@@ -348,7 +348,7 @@
 
     def test_acquire_node_handles_cpu_constraint(self):
         client = self.set_up_client_with_fake()
-        constraints = {"cpu": 2}
+        constraints = {"cpu": 2.0}
         client.acquire_node(constraints)
 
         cpu_count = client.post.params_used.get("cpu_count")
@@ -356,7 +356,7 @@
 
     def test_acquire_node_handles_mem_constraint(self):
         client = self.set_up_client_with_fake()
-        constraints = {"mem": 2048}
+        constraints = {"mem": 2048.0}
         client.acquire_node(constraints)
 
         mem = client.post.params_used.get("mem")