openstack/ironic
Code Issues Proposed changes

Clean up calls to get_chassis()

Browse Source 
The same work have already been done by Nodes (see 2192f2848e), so right
now the dbapi's are inconsistent across different objects. This patch
fixes it for Chassis.

This patch removes all direct calls to dbapi's get_chassis(). They
are replaced either by Chassis.get(), Chassis.get_by_id(), or
Chassis.get_by_uuid() calls.

Additional detail about the changes:

1) Break DBAPI get_chassis() into get_chassis_by_id() and get_chassis_by_uuid().
Let's be explicit about which type we're passing. This allows
Chassis.get_by_uuid() to work exactly how its named. Also, do not
return an object from the DB API calls. The Chassis object itself will
do the conversion.

2) Adds Chassis.get_by_id() to compliment the current
Chassis.get_by_uuid() for places you are querying by the ID.

3) Adds Chassis.get() so that you can still have a single method to use
to pass either and ID or a UUID.

4) Make all of the Chassis.get* calls set the 'context' so that you
can later just chassis.save() without passing it. Doc strings have been
updated to clarify what 'context' is for.

Co-Authored-By: Chris Behrens <cbehrens@codestud.com>
Related-Bug: #1314732
Change-Id: I5162c2843e15e538c1dc50dd5ac775298f8f0f9d
This commit is contained in:
Lucas Alvares Gomes 2014-07-23 12:14:21 +01:00
parent 9882c71687
commit e2e89126ef
6 changed files with 103 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
ironic/api/controllers/v1/node.py
View File

@ -271,8 +271,7 @@ class Node(base.APIBase):
def _set_chassis_uuid(self, value):
if value and self._chassis_uuid != value:
try:
chassis = objects.Chassis.get_by_uuid(pecan.request.context,
value)
chassis = objects.Chassis.get(pecan.request.context, value)
self._chassis_uuid = chassis.uuid
# NOTE(lucasagomes): Create the chassis_id attribute on-the-fly
# to satisfy the api -> rpc object

12
ironic/db/api.py
View File

@ -264,10 +264,18 @@ class Connection(object):
"""
@abc.abstractmethod
def get_chassis(self, chassis_id):
def get_chassis_by_id(self, chassis_id):
"""Return a chassis representation.
:param chassis_id: The id or the UUID of a chassis.
:param chassis_id: The id of a chassis.
:returns: A chassis.
"""
@abc.abstractmethod
def get_chassis_by_uuid(self, chassis_uuid):
"""Return a chassis representation.
:param chassis_uuid: The uuid of a chassis.
:returns: A chassis.
"""

19
ironic/db/sqlalchemy/api.py
View File

@ -169,8 +169,9 @@ class Connection(api.Connection):
filters = []
if 'chassis_uuid' in filters:
# get_chassis() to raise an exception if the chassis is not found
chassis_obj = self.get_chassis(filters['chassis_uuid'])
# get_chassis_by_uuid() to raise an exception if the chassis
# is not found
chassis_obj = self.get_chassis_by_uuid(filters['chassis_uuid'])
query = query.filter_by(chassis_id=chassis_obj.id)
if 'associated' in filters:
if filters['associated']:
@ -441,16 +442,20 @@ class Connection(api.Connection):
query.delete()
@objects.objectify(objects.Chassis)
def get_chassis(self, chassis_id):
query = model_query(models.Chassis)
query = add_identity_filter(query, chassis_id)
def get_chassis_by_id(self, chassis_id):
query = model_query(models.Chassis).filter_by(id=chassis_id)
try:
return query.one()
except NoResultFound:
raise exception.ChassisNotFound(chassis=chassis_id)
def get_chassis_by_uuid(self, chassis_uuid):
query = model_query(models.Chassis).filter_by(uuid=chassis_uuid)
try:
return query.one()
except NoResultFound:
raise exception.ChassisNotFound(chassis=chassis_uuid)
@objects.objectify(objects.Chassis)
def get_chassis_list(self, limit=None, marker=None,
sort_key=None, sort_dir=None):

65
ironic/objects/chassis.py
View File

@ -13,19 +13,26 @@
# License for the specific language governing permissions and limitations
# under the License.
from ironic.common import exception
from ironic.common import utils
from ironic.db import api as dbapi
from ironic.objects import base
from ironic.objects import utils
from ironic.objects import utils as obj_utils
class Chassis(base.IronicObject):
# Version 1.0: Initial version
# Version 1.1: Add get() and get_by_id() and make get_by_uuid()
# only work with a uuid
VERSION = '1.1'
dbapi = dbapi.get_instance()
fields = {
'id': int,
'uuid': utils.str_or_none,
'extra': utils.dict_or_none,
'description': utils.str_or_none,
'uuid': obj_utils.str_or_none,
'extra': obj_utils.dict_or_none,
'description': obj_utils.str_or_none,
}
@staticmethod
@ -43,24 +50,57 @@ class Chassis(base.IronicObject):
return chassis
@base.remotable_classmethod
def get_by_uuid(cls, context, uuid=None):
def get(cls, context, chassis_id):
"""Find a chassis based on its id or uuid and return a Chassis object.
:param chassis_id: the id *or* uuid of a chassis.
:returns: a :class:`Chassis` object.
"""
if utils.is_int_like(chassis_id):
return cls.get_by_id(context, chassis_id)
elif utils.is_uuid_like(chassis_id):
return cls.get_by_uuid(context, chassis_id)
else:
raise exception.InvalidIdentity(identity=chassis_id)
@base.remotable_classmethod
def get_by_id(cls, context, chassis_id):
"""Find a chassis based on its integer id and return a Chassis object.
:param chassis_id: the id of a chassis.
:returns: a :class:`Chassis` object.
"""
db_chassis = cls.dbapi.get_chassis_by_id(chassis_id)
chassis = Chassis._from_db_object(cls(), db_chassis)
# FIXME(comstud): Setting of the context should be moved to
# _from_db_object().
chassis._context = context
return chassis
@base.remotable_classmethod
def get_by_uuid(cls, context, uuid):
"""Find a chassis based on uuid and return a :class:`Chassis` object.
:param uuid: the uuid of a chassis.
:param context: Security context
:returns: a :class:`Chassis` object.
"""
db_chassis = cls.dbapi.get_chassis(uuid)
return Chassis._from_db_object(cls(), db_chassis)
db_chassis = cls.dbapi.get_chassis_by_uuid(uuid)
chassis = Chassis._from_db_object(cls(), db_chassis)
# FIXME(comstud): Setting of the context should be moved to
# _from_db_object().
chassis._context = context
return chassis
@base.remotable
def save(self, context):
def save(self, context=None):
"""Save updates to this Chassis.
Updates will be made column by column based on the result
of self.what_changed().
:param context: Security context
:param context: Security context. NOTE: This is only used
internally by the indirection_api.
"""
updates = self.obj_get_changes()
self.dbapi.update_chassis(self.uuid, updates)
@ -68,16 +108,17 @@ class Chassis(base.IronicObject):
self.obj_reset_changes()
@base.remotable
def refresh(self, context):
def refresh(self, context=None):
"""Loads and applies updates for this Chassis.
Loads a :class:`Chassis` with the same uuid from the database and
checks for updated attributes. Updates are applied from
the loaded chassis column by column, if there are any updates.
:param context: Security context
:param context: Security context. NOTE: This is only used
internally by the indirection_api.
"""
current = self.__class__.get_by_uuid(context, uuid=self.uuid)
current = self.__class__.get_by_uuid(self._context, uuid=self.uuid)
for field in self.fields:
if (hasattr(self, base.get_attrname(field)) and
self[field] != current[field]):

8
ironic/tests/db/test_chassis.py
View File

@ -52,19 +52,19 @@ class DbChassisTestCase(base.DbTestCase):
def test_get_chassis_by_id(self):
ch = self._create_test_chassis()
chassis = self.dbapi.get_chassis(ch['id'])
chassis = self.dbapi.get_chassis_by_id(ch['id'])
self.assertEqual(ch['uuid'], chassis.uuid)
def test_get_chassis_by_uuid(self):
ch = self._create_test_chassis()
chassis = self.dbapi.get_chassis(ch['uuid'])
chassis = self.dbapi.get_chassis_by_uuid(ch['uuid'])
self.assertEqual(ch['id'], chassis.id)
def test_get_chassis_that_does_not_exist(self):
self.assertRaises(exception.ChassisNotFound,
self.dbapi.get_chassis, 666)
self.dbapi.get_chassis_by_id, 666)
def test_update_chassis(self):
ch = self._create_test_chassis()
@ -87,7 +87,7 @@ class DbChassisTestCase(base.DbTestCase):
self.dbapi.destroy_chassis(ch['id'])
self.assertRaises(exception.ChassisNotFound,
self.dbapi.get_chassis, ch['id'])
self.dbapi.get_chassis_by_id, ch['id'])
def test_destroy_chassis_that_does_not_exist(self):
self.assertRaises(exception.ChassisNotFound,

30
ironic/tests/objects/test_chassis.py
View File

@ -15,6 +15,7 @@
import mock
from ironic.common import exception
from ironic.common import utils as ironic_utils
from ironic.db import api as db_api
from ironic.db.sqlalchemy import models
@ -31,19 +32,33 @@ class TestChassisObject(base.DbTestCase):
self.fake_chassis = utils.get_test_chassis()
self.dbapi = db_api.get_instance()
def test_load(self):
uuid = self.fake_chassis['uuid']
with mock.patch.object(self.dbapi, 'get_chassis',
def test_get_by_id(self):
chassis_id = self.fake_chassis['id']
with mock.patch.object(self.dbapi, 'get_chassis_by_id',
autospec=True) as mock_get_chassis:
mock_get_chassis.return_value = self.fake_chassis
objects.Chassis.get_by_uuid(self.context, uuid)
objects.Chassis.get(self.context, chassis_id)
mock_get_chassis.assert_called_once_with(chassis_id)
def test_get_by_uuid(self):
uuid = self.fake_chassis['uuid']
with mock.patch.object(self.dbapi, 'get_chassis_by_uuid',
autospec=True) as mock_get_chassis:
mock_get_chassis.return_value = self.fake_chassis
objects.Chassis.get(self.context, uuid)
mock_get_chassis.assert_called_once_with(uuid)
def test_get_bad_id_and_uuid(self):
self.assertRaises(exception.InvalidIdentity,
objects.Chassis.get, self.context, 'not-a-uuid')
def test_save(self):
uuid = self.fake_chassis['uuid']
with mock.patch.object(self.dbapi, 'get_chassis',
with mock.patch.object(self.dbapi, 'get_chassis_by_uuid',
autospec=True) as mock_get_chassis:
mock_get_chassis.return_value = self.fake_chassis
with mock.patch.object(self.dbapi, 'update_chassis',
@ -63,8 +78,9 @@ class TestChassisObject(base.DbTestCase):
returns = [dict(self.fake_chassis, uuid=uuid),
dict(self.fake_chassis, uuid=new_uuid)]
expected = [mock.call(uuid), mock.call(uuid)]
with mock.patch.object(self.dbapi, 'get_chassis', side_effect=returns,
autospec=True) as mock_get_chassis:
with mock.patch.object(self.dbapi, 'get_chassis_by_uuid',
side_effect=returns, autospec=True) \
as mock_get_chassis:
c = objects.Chassis.get_by_uuid(self.context, uuid)
self.assertEqual(uuid, c.uuid)
c.refresh()