From 8a01731d4e68152e9f575bea78b2baa085986726 Mon Sep 17 00:00:00 2001 From: Eric Pendergrass Date: Tue, 29 Jul 2014 08:52:20 -0700 Subject: Verify alarm found before modifying Current behavior is to retrieve alarm by id and conduct operations on the object. If the tenant doesn't own the alarm or isn't admin, the user will receive the message: 'NoneType' object has no attribute 'to_dict' Above message doesn't provide any useful diagnostic information and indicates a programming error since an unexpected None-type is encountered and not handled. This change verifies the alarm is found before using the object. If alarm not found it prints the same message for a not found Alarm as other PUT operations like alarm-state-set: Alarm not found: . This message is more useful for diagnosis and gets rid of the uncaught None-type error. Change-Id: I66abcd4498b24ac7cadcf29fe3ced3fcda08458c Closes-Bug: #1348387 --- ceilometerclient/common/base.py | 6 +++++- ceilometerclient/tests/v2/test_alarms.py | 20 ++++++++++++++++++++ ceilometerclient/v2/alarms.py | 6 +++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/ceilometerclient/common/base.py b/ceilometerclient/common/base.py index 7d4be24..4129176 100644 --- a/ceilometerclient/common/base.py +++ b/ceilometerclient/common/base.py @@ -19,6 +19,7 @@ Base utilities to build API operation managers and objects on top of. import copy +from ceilometerclient import exc from ceilometerclient.openstack.common.apiclient import base # Python 2.4 compat @@ -55,7 +56,10 @@ class Manager(object): def _list(self, url, response_key=None, obj_class=None, body=None, expect_single=False): - body = self.api.get(url).json() + resp = self.api.get(url) + if not resp.content: + raise exc.HTTPNotFound + body = resp.json() if obj_class is None: obj_class = self.resource_class diff --git a/ceilometerclient/tests/v2/test_alarms.py b/ceilometerclient/tests/v2/test_alarms.py index c3aa6d5..7f0d8bf 100644 --- a/ceilometerclient/tests/v2/test_alarms.py +++ b/ceilometerclient/tests/v2/test_alarms.py @@ -21,6 +21,7 @@ import six from six.moves import xrange # noqa import testtools +from ceilometerclient import exc from ceilometerclient.openstack.common.apiclient import client from ceilometerclient.openstack.common.apiclient import fake_client from ceilometerclient.v2 import alarms @@ -207,6 +208,17 @@ fixtures = { None, ), }, + '/v2/alarms/unk-alarm-id': + { + 'GET': ( + {}, + None, + ), + 'PUT': ( + {}, + None, + ), + }, '/v2/alarms/alarm-id/state': { 'PUT': ( @@ -380,6 +392,14 @@ class AlarmManagerTest(testtools.TestCase): self.http_client.assert_called(*expect_get_2, pos=1) self.assertEqual('alarm', state) + def test_update_missing(self): + alarm = None + try: + alarm = self.mgr.update(alarm_id='unk-alarm-id', **UPDATE_ALARM) + except exc.CommandError: + pass + self.assertEqual(alarm, None) + def test_delete_from_alarm_class(self): alarm = self.mgr.get(alarm_id='alarm-id') self.assertIsNotNone(alarm) diff --git a/ceilometerclient/v2/alarms.py b/ceilometerclient/v2/alarms.py index 341cbb7..694b5c0 100644 --- a/ceilometerclient/v2/alarms.py +++ b/ceilometerclient/v2/alarms.py @@ -84,6 +84,7 @@ class AlarmManager(base.Manager): return self._list(self._path(alarm_id), expect_single=True)[0] except IndexError: return None + except exc.HTTPNotFound: # When we try to get deleted alarm HTTPNotFound occurs # or when alarm doesn't exists this exception don't must @@ -156,7 +157,10 @@ class AlarmManager(base.Manager): def update(self, alarm_id, **kwargs): self._compat_legacy_alarm_kwargs(kwargs) - updated = self.get(alarm_id).to_dict() + alarm = self.get(alarm_id) + if alarm is None: + raise exc.CommandError('Alarm not found: %s' % alarm_id) + updated = alarm.to_dict() updated['time_constraints'] = self._merge_time_constraints( updated.get('time_constraints', []), kwargs) kwargs = dict((k, v) for k, v in kwargs.items() -- cgit v1.2.1