Code review comment for lp:~gz/pyjuju/maas_int_constraints_1061286

Revision history for this message
Martin Packman (gz) wrote :

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 = client.post.params_used.get("mem")

« Back to merge proposal