From a540180790c474f152ff5533f796635c1f7d764c Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Wed, 12 Apr 2017 11:10:48 -0500 Subject: [PATCH] Remove get_schema_helper retry/try_add_manager parameters With enable_connection_uri taking optional arguments for handling combinations of retrying and adding missing manager connections, it was ugly trying to pass that information through get_schema_helper. Neutron is the only user of the extra options to get_schema_helper and both OVN and dragonflow have had to work around issues with get_schema_helper and enable_connection_uri. In addition, enable_connection_uri is Open_vSwitch schema specific. So it makes sense to move the burden of adding any retrying or modifying ovsdb-server connections on the user of ovsdbapp. Change-Id: I886ccf2aac7ec5b09b47b308933b81d3d09c3284 --- ovsdbapp/backend/ovs_idl/connection.py | 3 +- ovsdbapp/backend/ovs_idl/idlutils.py | 32 +++++-------------- .../open_vswitch}/helpers.py | 0 .../schema/open_vswitch/test_impl_idl.py | 2 +- .../unit/backend/ovs_idl/test_connection.py | 20 ------------ .../unit/backend/ovs_idl/test_helpers.py | 2 +- requirements.txt | 1 - 7 files changed, 11 insertions(+), 49 deletions(-) rename ovsdbapp/{backend/ovs_idl => schema/open_vswitch}/helpers.py (100%) diff --git a/ovsdbapp/backend/ovs_idl/connection.py b/ovsdbapp/backend/ovs_idl/connection.py index 5045ba12..d76a3cc4 100644 --- a/ovsdbapp/backend/ovs_idl/connection.py +++ b/ovsdbapp/backend/ovs_idl/connection.py @@ -125,8 +125,7 @@ class Connection(object): message="Use idlutils.get_schema_helper(conn, schema, retry=True)") def get_schema_helper(self): """Retrieve the schema helper object from OVSDB""" - return idlutils.get_schema_helper(self.connection, self.schema_name, - retry=True) + return idlutils.get_schema_helper(self.connection, self.schema_name) @removals.remove( version='Ocata', removal_version='Pike', diff --git a/ovsdbapp/backend/ovs_idl/idlutils.py b/ovsdbapp/backend/ovs_idl/idlutils.py index b143e923..2bb2cc68 100644 --- a/ovsdbapp/backend/ovs_idl/idlutils.py +++ b/ovsdbapp/backend/ovs_idl/idlutils.py @@ -22,10 +22,8 @@ from ovs import jsonrpc from ovs import poller from ovs import stream import six -import tenacity from ovsdbapp import api -from ovsdbapp.backend.ovs_idl import helpers from ovsdbapp import exceptions @@ -98,7 +96,14 @@ class ExceptionResult(object): self.tb = tb -def _get_schema_helper(connection, schema_name): +def get_schema_helper(connection, schema_name): + """Create a schema helper object by querying an ovsdb-server + + :param connection: The ovsdb-server connection string + :type connection: string + :param schema_name: The schema on the server to pull + :type schema_name: string + """ err, strm = stream.Stream.open_block( stream.Stream.open(connection)) if err: @@ -116,27 +121,6 @@ def _get_schema_helper(connection, schema_name): return idl.SchemaHelper(None, resp.result) -def get_schema_helper(connection, schema_name, - retry=True, try_add_manager=True): - try: - return _get_schema_helper(connection, schema_name) - except Exception: - if not retry: - raise - # We may have failed due to set-manager not being called - if try_add_manager: - helpers.enable_connection_uri(connection) - - # There is a small window for a race, so retry up to a second - @tenacity.retry(wait=tenacity.wait_exponential(multiplier=0.01), - stop=tenacity.stop_after_delay(1), - reraise=True) - def do_get_schema_helper(): - return _get_schema_helper(connection, schema_name) - - return do_get_schema_helper() - - def wait_for_change(_idl, timeout, seqno=None): if seqno is None: seqno = _idl.change_seqno diff --git a/ovsdbapp/backend/ovs_idl/helpers.py b/ovsdbapp/schema/open_vswitch/helpers.py similarity index 100% rename from ovsdbapp/backend/ovs_idl/helpers.py rename to ovsdbapp/schema/open_vswitch/helpers.py diff --git a/ovsdbapp/tests/functional/schema/open_vswitch/test_impl_idl.py b/ovsdbapp/tests/functional/schema/open_vswitch/test_impl_idl.py index c2c260b3..07a69927 100644 --- a/ovsdbapp/tests/functional/schema/open_vswitch/test_impl_idl.py +++ b/ovsdbapp/tests/functional/schema/open_vswitch/test_impl_idl.py @@ -25,7 +25,7 @@ from ovsdbapp.tests import utils def default_idl_factory(): helper = idlutils.get_schema_helper(constants.DEFAULT_OVSDB_CONNECTION, - 'Open_vSwitch', retry=False) + 'Open_vSwitch') helper.register_all() return idl.Idl(constants.DEFAULT_OVSDB_CONNECTION, helper) diff --git a/ovsdbapp/tests/unit/backend/ovs_idl/test_connection.py b/ovsdbapp/tests/unit/backend/ovs_idl/test_connection.py index 585ca532..11bbbda7 100644 --- a/ovsdbapp/tests/unit/backend/ovs_idl/test_connection.py +++ b/ovsdbapp/tests/unit/backend/ovs_idl/test_connection.py @@ -80,23 +80,3 @@ class TestOVSNativeConnection(base.TestCase): idl_instance = idl_class.return_value self.connection.start() self.assertEqual(idl_instance, self.connection.idl) - - @mock.patch.object(connection, 'threading') - @mock.patch.object(idlutils, 'wait_for_change') - @mock.patch.object(connection, 'idl') - @mock.patch.object(idlutils.helpers, 'enable_connection_uri') - @mock.patch.object(idlutils, '_get_schema_helper') - def test_do_get_schema_helper_retry(self, mock_get_schema_helper, - mock_enable_conn, - mock_idl, - mock_wait_for_change, - mock_threading): - mock_helper = mock.Mock() - # raise until 3rd retry attempt - mock_get_schema_helper.side_effect = [Exception(), Exception(), - mock_helper] - conn = connection.Connection( - mock.Mock(), mock.Mock(), mock.Mock()) - conn.start() - self.assertEqual(3, len(mock_get_schema_helper.mock_calls)) - mock_helper.register_all.assert_called_once_with() diff --git a/ovsdbapp/tests/unit/backend/ovs_idl/test_helpers.py b/ovsdbapp/tests/unit/backend/ovs_idl/test_helpers.py index 397a1a82..02e492fb 100644 --- a/ovsdbapp/tests/unit/backend/ovs_idl/test_helpers.py +++ b/ovsdbapp/tests/unit/backend/ovs_idl/test_helpers.py @@ -12,7 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. -from ovsdbapp.backend.ovs_idl import helpers +from ovsdbapp.schema.open_vswitch import helpers from ovsdbapp.tests import base diff --git a/requirements.txt b/requirements.txt index 577c1598..6d719778 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,4 +4,3 @@ ovs>=2.7.0 # Apache-2.0 pbr!=2.1.0,>=2.0.0 # Apache-2.0 -tenacity>=3.2.1 # Apache-2.0