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: <alarm_id>.

This message is more useful for diagnosis and gets rid of the uncaught
None-type error.

Change-Id: I66abcd4498b24ac7cadcf29fe3ced3fcda08458c
Closes-Bug: #1348387
This commit is contained in:
Eric Pendergrass
2014-07-29 08:52:20 -07:00
parent 11134c5675
commit 8a01731d4e
3 changed files with 30 additions and 2 deletions

View File

@@ -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

View File

@@ -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)

View File

@@ -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()