diff options
| author | Mark McLoughlin <markmc@redhat.com> | 2012-12-05 23:05:59 +0000 |
|---|---|---|
| committer | Mark McLoughlin <markmc@redhat.com> | 2012-12-05 23:05:59 +0000 |
| commit | 2a33f8d8cefdbde50f89e6d4084d513eadf3a97a (patch) | |
| tree | a3b9b4143da9e1e8768d8aab5fbb0c316beed599 | |
| parent | 255692feea3eee12bfc763f75fc8f3dabdbe9ba5 (diff) | |
| download | nova-2a33f8d8cefdbde50f89e6d4084d513eadf3a97a.tar.gz nova-2a33f8d8cefdbde50f89e6d4084d513eadf3a97a.tar.xz nova-2a33f8d8cefdbde50f89e6d4084d513eadf3a97a.zip | |
Fix positional arg swallow decorator
The update() method in the hosts extension is decorated with @check_host
but the function returned by decorator takes a kwarg which shadows the
'body' positional arg of the update() method.
i.e. if you call:
controller.update(req, id, body)
then the body arg gets passed to the wrapper as its service kwarg and
never passed through to update(). We can see how the tests are all
passing body as a kwarg to avoid this.
Remove the service arg from check_host() but also from _list_hosts()
since the only place it is used is in the tests.
Change-Id: I12aab582c15c25fd35ecda005341291450c7efdc
| -rw-r--r-- | nova/api/openstack/compute/contrib/hosts.py | 9 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/contrib/test_hosts.py | 19 |
2 files changed, 10 insertions, 18 deletions
diff --git a/nova/api/openstack/compute/contrib/hosts.py b/nova/api/openstack/compute/contrib/hosts.py index 8982f5724..0d2b68f11 100644 --- a/nova/api/openstack/compute/contrib/hosts.py +++ b/nova/api/openstack/compute/contrib/hosts.py @@ -93,7 +93,7 @@ class HostUpdateDeserializer(wsgi.XMLDeserializer): return dict(body=updates) -def _list_hosts(req, service=None): +def _list_hosts(req): """Returns a summary list of hosts, optionally filtering by service type. """ @@ -108,16 +108,13 @@ def _list_hosts(req, service=None): for host in services: hosts.append({"host_name": host['host'], 'service': host['topic'], 'zone': host['availability_zone']}) - if service: - hosts = [host for host in hosts - if host["service"] == service] return hosts def check_host(fn): """Makes sure that the host exists.""" - def wrapped(self, req, id, service=None, *args, **kwargs): - listed_hosts = _list_hosts(req, service) + def wrapped(self, req, id, *args, **kwargs): + listed_hosts = _list_hosts(req) hosts = [h["host_name"] for h in listed_hosts] if id in hosts: return fn(self, req, id, *args, **kwargs) diff --git a/nova/tests/api/openstack/compute/contrib/test_hosts.py b/nova/tests/api/openstack/compute/contrib/test_hosts.py index f56170a06..b91715190 100644 --- a/nova/tests/api/openstack/compute/contrib/test_hosts.py +++ b/nova/tests/api/openstack/compute/contrib/test_hosts.py @@ -124,7 +124,7 @@ class HostTestCase(test.TestCase): def _test_host_update(self, host, key, val, expected_value): body = {key: val} - result = self.controller.update(self.req, host, body=body) + result = self.controller.update(self.req, host, body) self.assertEqual(result[key], expected_value) def test_list_hosts(self): @@ -132,11 +132,6 @@ class HostTestCase(test.TestCase): hosts = os_hosts._list_hosts(self.req) self.assertEqual(hosts, HOST_LIST['hosts']) - compute_hosts = os_hosts._list_hosts(self.req, "compute") - expected = [host for host in HOST_LIST['hosts'] - if host["service"] == "compute"] - self.assertEqual(compute_hosts, expected) - def test_list_hosts_with_zone(self): req = FakeRequestWithNovaZone() hosts = os_hosts._list_hosts(req) @@ -173,31 +168,31 @@ class HostTestCase(test.TestCase): def test_bad_status_value(self): bad_body = {"status": "bad"} self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", body=bad_body) + self.req, "host_c1", bad_body) bad_body2 = {"status": "disablabc"} self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", body=bad_body2) + self.req, "host_c1", bad_body2) def test_bad_update_key(self): bad_body = {"crazy": "bad"} self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", body=bad_body) + self.req, "host_c1", bad_body) def test_bad_update_key_and_correct_udpate_key(self): bad_body = {"status": "disable", "crazy": "bad"} self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, - self.req, "host_c1", body=bad_body) + self.req, "host_c1", bad_body) def test_good_udpate_keys(self): body = {"status": "disable", "maintenance_mode": "enable"} - result = self.controller.update(self.req, 'host_c1', body=body) + result = self.controller.update(self.req, 'host_c1', body) self.assertEqual(result["host"], "host_c1") self.assertEqual(result["status"], "disabled") self.assertEqual(result["maintenance_mode"], "on_maintenance") def test_bad_host(self): self.assertRaises(webob.exc.HTTPNotFound, self.controller.update, - self.req, "bogus_host_name", body={"status": "disable"}) + self.req, "bogus_host_name", {"status": "disable"}) def test_show_forbidden(self): self.req.environ["nova.context"].is_admin = False |
