openstack/python-openstackclient
Code Issues Proposed changes

Structure FindFloatingIP() to work without ip_cache

Browse Source 
Currently we have an ip_cache returned from _find_floating_ip() which
is generally ignored as it is not always required.

This patch removes the need of ip_cache in _find_floating_ip().

Co-Authored-By: Sindhu Devale<sindhu.devale@intel.com>

Change-Id: I8b92271185f82f275fa73adad03e9dad70be70e4
This commit is contained in:
Reedip 2017-03-21 08:05:54 +00:00 committed by Sindhu Devale
parent 8a1f4b6592
commit 0e42ea3ae3
2 changed files with 13 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
openstackclient/network/v2/floating_ip.py
View File

@ -83,7 +83,6 @@ def _get_attrs(client_manager, parsed_args):
def _find_floating_ip( def _find_floating_ip(
session, session,
ip_cache,
name_or_id, name_or_id,
ignore_missing=True, ignore_missing=True,
**params **params
@ -93,11 +92,11 @@ def _find_floating_ip(
The SDK's find_ip() can only locate a floating IP by ID so we have The SDK's find_ip() can only locate a floating IP by ID so we have
to do this ourselves. to do this ourselves.
""" """
def _get_one_match(name_or_id): def _get_one_match(name_or_id):
"""Given a list of results, return the match""" """Given a list of results, return the match"""
the_result = None the_result = None
for maybe_result in ip_cache: ip_list = list(_floating_ip.FloatingIP.list(session, **params))
for maybe_result in ip_list:
id_value = maybe_result.id id_value = maybe_result.id
ip_value = maybe_result.floating_ip_address ip_value = maybe_result.floating_ip_address
@ -116,19 +115,16 @@ def _find_floating_ip(
# Try to short-circuit by looking directly for a matching ID. # Try to short-circuit by looking directly for a matching ID.
try: try:
match = _floating_ip.FloatingIP.existing(id=name_or_id, **params) match = _floating_ip.FloatingIP.existing(id=name_or_id, **params)
return (match.get(session), ip_cache) return match.get(session)
except sdk_exceptions.NotFoundException: except sdk_exceptions.NotFoundException:
pass pass
if len(ip_cache) == 0:
ip_cache = list(_floating_ip.FloatingIP.list(session, **params))
result = _get_one_match(name_or_id) result = _get_one_match(name_or_id)
if result is not None: if result is not None:
return (result, ip_cache) return result
if ignore_missing: if ignore_missing:
return (None, ip_cache) return None
raise sdk_exceptions.ResourceNotFound( raise sdk_exceptions.ResourceNotFound(
"No %s found for %s" % (_floating_ip.FloatingIP.__name__, name_or_id)) "No %s found for %s" % (_floating_ip.FloatingIP.__name__, name_or_id))
@ -240,9 +236,8 @@ class DeleteFloatingIP(common.NetworkAndComputeDelete):
return parser return parser
def take_action_network(self, client, parsed_args): def take_action_network(self, client, parsed_args):
(obj, self.ip_cache) = _find_floating_ip( obj = _find_floating_ip(
self.app.client_manager.sdk_connection.session, self.app.client_manager.sdk_connection.session,
self.ip_cache,
self.r, self.r,
ignore_missing=False, ignore_missing=False,
) )
@ -255,11 +250,6 @@ class DeleteFloatingIP(common.NetworkAndComputeDelete):
def take_action(self, parsed_args): def take_action(self, parsed_args):
"""Implements a naive cache for the list of floating IPs""" """Implements a naive cache for the list of floating IPs"""
# NOTE(dtroyer): This really only prevents multiple list()
# calls when performing multiple resource deletes
# in a single command. In an interactive session
# each delete command will call list().
self.ip_cache = []
super(DeleteFloatingIP, self).take_action(parsed_args) super(DeleteFloatingIP, self).take_action(parsed_args)
@ -459,9 +449,6 @@ class ListIPFloating(ListFloatingIP):
class ShowFloatingIP(common.NetworkAndComputeShowOne): class ShowFloatingIP(common.NetworkAndComputeShowOne):
_description = _("Display floating IP details") _description = _("Display floating IP details")
# ip_cache is unused here but is a side effect of _find_floating_ip()
ip_cache = []
def update_parser_common(self, parser): def update_parser_common(self, parser):
parser.add_argument( parser.add_argument(
'floating_ip', 'floating_ip',
@ -471,9 +458,8 @@ class ShowFloatingIP(common.NetworkAndComputeShowOne):
return parser return parser
def take_action_network(self, client, parsed_args): def take_action_network(self, client, parsed_args):
(obj, self.ip_cache) = _find_floating_ip( obj = _find_floating_ip(
self.app.client_manager.sdk_connection.session, self.app.client_manager.sdk_connection.session,
[],
parsed_args.floating_ip, parsed_args.floating_ip,
ignore_missing=False, ignore_missing=False,
) )

18
openstackclient/tests/unit/network/v2/test_floating_ip.py
View File

@ -218,8 +218,8 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
) )
def test_floating_ip_delete(self, find_floating_ip_mock): def test_floating_ip_delete(self, find_floating_ip_mock):
find_floating_ip_mock.side_effect = [ find_floating_ip_mock.side_effect = [
(self.floating_ips[0], []), self.floating_ips[0],
(self.floating_ips[1], []), self.floating_ips[1],
] ]
arglist = [ arglist = [
self.floating_ips[0].id, self.floating_ips[0].id,
@ -233,7 +233,6 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
find_floating_ip_mock.assert_called_once_with( find_floating_ip_mock.assert_called_once_with(
mock.ANY, mock.ANY,
[],
self.floating_ips[0].id, self.floating_ips[0].id,
ignore_missing=False, ignore_missing=False,
) )
@ -246,8 +245,8 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
) )
def test_floating_ip_delete_multi(self, find_floating_ip_mock): def test_floating_ip_delete_multi(self, find_floating_ip_mock):
find_floating_ip_mock.side_effect = [ find_floating_ip_mock.side_effect = [
(self.floating_ips[0], []), self.floating_ips[0],
(self.floating_ips[1], []), self.floating_ips[1],
] ]
arglist = [] arglist = []
verifylist = [] verifylist = []
@ -264,13 +263,11 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
calls = [ calls = [
call( call(
mock.ANY, mock.ANY,
[],
self.floating_ips[0].id, self.floating_ips[0].id,
ignore_missing=False, ignore_missing=False,
), ),
call( call(
mock.ANY, mock.ANY,
[],
self.floating_ips[1].id, self.floating_ips[1].id,
ignore_missing=False, ignore_missing=False,
), ),
@ -289,7 +286,7 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
) )
def test_floating_ip_delete_multi_exception(self, find_floating_ip_mock): def test_floating_ip_delete_multi_exception(self, find_floating_ip_mock):
find_floating_ip_mock.side_effect = [ find_floating_ip_mock.side_effect = [
(self.floating_ips[0], []), self.floating_ips[0],
exceptions.CommandError, exceptions.CommandError,
] ]
arglist = [ arglist = [
@ -310,13 +307,11 @@ class TestDeleteFloatingIPNetwork(TestFloatingIPNetwork):
find_floating_ip_mock.assert_any_call( find_floating_ip_mock.assert_any_call(
mock.ANY, mock.ANY,
[],
self.floating_ips[0].id, self.floating_ips[0].id,
ignore_missing=False, ignore_missing=False,
) )
find_floating_ip_mock.assert_any_call( find_floating_ip_mock.assert_any_call(
mock.ANY, mock.ANY,
[],
'unexist_floating_ip', 'unexist_floating_ip',
ignore_missing=False, ignore_missing=False,
) )
@ -584,7 +579,7 @@ class TestShowFloatingIPNetwork(TestFloatingIPNetwork):
"floating_ip._find_floating_ip" "floating_ip._find_floating_ip"
) )
def test_floating_ip_show(self, find_floating_ip_mock): def test_floating_ip_show(self, find_floating_ip_mock):
find_floating_ip_mock.return_value = (self.floating_ip, []) find_floating_ip_mock.return_value = self.floating_ip
arglist = [ arglist = [
self.floating_ip.id, self.floating_ip.id,
] ]
@ -597,7 +592,6 @@ class TestShowFloatingIPNetwork(TestFloatingIPNetwork):
find_floating_ip_mock.assert_called_once_with( find_floating_ip_mock.assert_called_once_with(
mock.ANY, mock.ANY,
[],
self.floating_ip.id, self.floating_ip.id,
ignore_missing=False, ignore_missing=False,
) )