From 74d56d187a730e832c815141341d4f5f9a2171b1 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 2 Apr 2014 13:33:28 -0700 Subject: [PATCH] Remove a DB query for get_ports_by_node() This renames get_ports_by_node() to get_ports_by_node_id() and changes the node_id argument to always need to be the integer ID. This removes the need for get_ports_by_node() to query the DB for UUID -> ID conversion. By doing this, we can save a DB query in the cases where we already have the Node integer ID. This is needed to help set up the next patch to clean up calls to get_node(). There's now a case in port API controller where we need to lookup the Node by UUID just to get its ID to be passed into this call. Until we implement the Node lazy-loading code, we fetch the full Node, but this isn't any worse than before for this case, as I'm just moving it from the DB layer. I've noted this with a FIXME and it will be made more efficient in the future. Change-Id: I9993c0c87794171fb971524ac5c30490b3e33716 --- ironic/api/controllers/v1/port.py | 13 +++++++++---- ironic/conductor/task_manager.py | 2 +- ironic/db/api.py | 6 +++--- ironic/db/sqlalchemy/api.py | 8 +++----- ironic/tests/db/test_ports.py | 17 +++-------------- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 51cdea503f..554da81929 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -175,10 +175,15 @@ class PortsController(rest.RestController): marker) if node_uuid: - ports = pecan.request.dbapi.get_ports_by_node(node_uuid, limit, - marker_obj, - sort_key=sort_key, - sort_dir=sort_dir) + # FIXME(comstud): Since all we need is the node ID, we can + # make this more efficient by only querying + # for that column. This will get cleaned up + # as we move to the object interface. + node = pecan.request.dbapi.get_node(node_uuid) + ports = pecan.request.dbapi.get_ports_by_node_id(node.id, limit, + marker_obj, + sort_key=sort_key, + sort_dir=sort_dir) elif address: ports = self._get_ports_by_address(address) else: diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index 6d3ae9da93..69389cc050 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -165,7 +165,7 @@ class TaskManager(object): locked_node_list.append(node.id) else: node = self.dbapi.get_node(id) - ports = self.dbapi.get_ports_by_node(id) + ports = self.dbapi.get_ports_by_node_id(node.id) driver = driver_factory.get_driver(driver_name or node.driver) self.resources.append(NodeResource(node, ports, driver)) diff --git a/ironic/db/api.py b/ironic/db/api.py index 340bffbd93..e880ef343c 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -208,11 +208,11 @@ class Connection(object): """ @abc.abstractmethod - def get_ports_by_node(self, node_id, limit=None, marker=None, - sort_key=None, sort_dir=None): + def get_ports_by_node_id(self, node_id, limit=None, marker=None, + sort_key=None, sort_dir=None): """List all the ports for a given node. - :param node_id: The id or uuid of a node. + :param node_id: The integer node ID. :param limit: Maximum number of ports to return. :param marker: the last item of the previous page; we return the next result set. diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index e3a2c752d9..0483fe3c73 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -403,12 +403,10 @@ class Connection(api.Connection): sort_key, sort_dir) @objects.objectify(objects.Port) - def get_ports_by_node(self, node_id, limit=None, marker=None, - sort_key=None, sort_dir=None): - # get_node() to raise an exception if the node is not found - node_obj = self.get_node(node_id) + def get_ports_by_node_id(self, node_id, limit=None, marker=None, + sort_key=None, sort_dir=None): query = model_query(models.Port) - query = query.filter_by(node_id=node_obj.id) + query = query.filter_by(node_id=node_id) return _paginate_query(models.Port, limit, marker, sort_key, sort_dir, query) diff --git a/ironic/tests/db/test_ports.py b/ironic/tests/db/test_ports.py index 2e96bd0032..b129091f4b 100644 --- a/ironic/tests/db/test_ports.py +++ b/ironic/tests/db/test_ports.py @@ -73,23 +73,12 @@ class DbPortTestCase(base.DbTestCase): def test_get_ports_by_node_id(self): p = db_utils.get_test_port(node_id=self.n.id) self.dbapi.create_port(p) - res = self.dbapi.get_ports_by_node(self.n.id) + res = self.dbapi.get_ports_by_node_id(self.n.id) self.assertEqual(self.p['address'], res[0].address) - def test_get_ports_by_node_uuid(self): - p = db_utils.get_test_port(node_id=self.n.id) - self.dbapi.create_port(p) - res = self.dbapi.get_ports_by_node(self.n.uuid) - self.assertEqual(self.p['address'], res[0].address) - - def test_get_ports_by_node_that_does_not_exist(self): + def test_get_ports_by_node_id_that_does_not_exist(self): self.dbapi.create_port(self.p) - self.assertRaises(exception.NodeNotFound, - self.dbapi.get_ports_by_node, - 99) - self.assertRaises(exception.NodeNotFound, - self.dbapi.get_ports_by_node, - '12345678-9999-0000-aaaa-123456789012') + self.assertEqual([], self.dbapi.get_ports_by_node_id(99)) def test_destroy_port(self): self.dbapi.create_port(self.p)