Pass a connection to OvsdbIdl.__init__()

Originally, OvsdbIdl took a context object which in practice was
a Neutron ovs_lib.BaseOVS object. It did this because it was possible
with the old ovs-vsctl implementation to change the 'vsctl_timeout'
attribute and have that immediately reflected in the timeout value
passed to ovs-vsctl. There were some unit tests that used this. So,
when writing the CLI implementation of the OVSDB api, this context
was passed in so that the existing behavior would be honored.

With the native ovsdb implementation, this context.vsctl_timeout value
was passed to the Connection object each Transaction when it
was created. This means that the timeout, if changed on the context
object could end up being different in Connection.run and
Transaction.do_commit.

Given that non-Neutron users will never need a 'context' and that
this library does not include the ovs-vsctl implmentation, this patch
removes 'context' from the API definition and the OvsdbIdl class now
requires a 'connection' attribute and no timeout. Neutron will instead
define factory functions that fill in the appropriate attributes when
its api.get() is called.

Change-Id: Ic61c5bc988e5e2f89c92cce0a2e8169407934af9

ovsdbapp/api.py

@@ -56,8 +56,7 @@ class Transaction(object):
 
 @six.add_metaclass(abc.ABCMeta)
 class API(object):
-    def __init__(self, context):
-        self.context = context
+    def __init__(self):
         self._nested_txn = None
 
     @abc.abstractmethod

ovsdbapp/backend/ovs_idl/transaction.py

@@ -26,7 +26,7 @@ LOG = logging.getLogger(__name__)
 
 
 class Transaction(api.Transaction):
-    def __init__(self, api, ovsdb_connection, timeout,
+    def __init__(self, api, ovsdb_connection, timeout=None,
                  check_error=False, log_errors=True):
         self.api = api
         self.check_error = check_error
@@ -34,7 +34,7 @@ class Transaction(api.Transaction):
         self.commands = []
         self.results = Queue.Queue(1)
         self.ovsdb_connection = ovsdb_connection
-        self.timeout = timeout
+        self.timeout = timeout or ovsdb_connection.timeout
 
     def __str__(self):
         return ", ".join(str(cmd) for cmd in self.commands)

ovsdbapp/schema/open_vswitch/impl_idl.py

@@ -14,7 +14,6 @@
 
 import logging
 
-from ovsdbapp.backend.ovs_idl import connection
 from ovsdbapp.backend.ovs_idl import transaction
 from ovsdbapp import exceptions
 from ovsdbapp.schema.open_vswitch import api
@@ -75,19 +74,11 @@ class OvsVsctlTransaction(transaction.Transaction):
 
 class OvsdbIdl(api.API):
 
-    ovsdb_connection = None
-
-    def __init__(self, context):
-        super(OvsdbIdl, self).__init__(context)
-        # TODO(twilson) Move to idl_factory, ensure neutron adds
-        #               ovsdb_connection attribute to BaseOVS
-        if not OvsdbIdl.ovsdb_connection:
-            OvsdbIdl.ovsdb_connection = connection.Connection(
-                connection=context.ovsdb_connection,
-                timeout=context.vsctl_timeout,
-                schema_name='Open_vSwitch')
-        OvsdbIdl.ovsdb_connection.start()
-        self.idl = OvsdbIdl.ovsdb_connection.idl
+    def __init__(self, connection):
+        super(OvsdbIdl, self).__init__()
+        self.connection = connection
+        self.connection.start()
+        self.idl = self.connection.idl
 
     @property
     def _tables(self):
@@ -98,9 +89,9 @@ class OvsdbIdl(api.API):
         return list(self._tables['Open_vSwitch'].rows.values())[0]
 
     def create_transaction(self, check_error=False, log_errors=True, **kwargs):
-        return OvsVsctlTransaction(self, OvsdbIdl.ovsdb_connection,
-                                   self.context.vsctl_timeout,
-                                   check_error, log_errors)
+        return OvsVsctlTransaction(self, self.connection,
+                                   check_error=check_error,
+                                   log_errors=log_errors)
 
     def add_manager(self, connection_uri):
         return cmd.AddManagerCommand(self, connection_uri)

ovsdbapp/tests/functional/schema/open_vswitch/test_impl_idl.py

@@ -13,22 +13,32 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from ovs.db import idl
+
+from ovsdbapp.backend.ovs_idl import connection
+from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp import constants
 from ovsdbapp.schema.open_vswitch import impl_idl
 from ovsdbapp.tests import base
 from ovsdbapp.tests import utils
 
 
-class DefaultContext(object):
-    ovsdb_connection = constants.DEFAULT_OVSDB_CONNECTION
-    vsctl_timeout = constants.DEFAULT_TIMEOUT
+def default_idl_factory():
+    helper = idlutils.get_schema_helper(constants.DEFAULT_OVSDB_CONNECTION,
+                                        'Open_vSwitch', retry=False)
+    helper.register_all()
+    return idl.Idl(constants.DEFAULT_OVSDB_CONNECTION, helper)
+
+
+ovsdb_connection = connection.Connection(
+    idl_factory=default_idl_factory, timeout=constants.DEFAULT_TIMEOUT)
 
 
 class TestOvsdbIdl(base.TestCase):
 
     def setUp(self):
         super(TestOvsdbIdl, self).setUp()
-        self.api = impl_idl.OvsdbIdl(DefaultContext())
+        self.api = impl_idl.OvsdbIdl(ovsdb_connection)
         self.brname = utils.get_rand_device_name()
         # Destroying the bridge cleans up most things created by tests
         cleanup_cmd = self.api.del_br(self.brname)

ovsdbapp/tests/unit/test_api.py

@@ -38,7 +38,7 @@ class TestingAPI(api.API):
 class TransactionTestCase(base.TestCase):
     def setUp(self):
         super(TransactionTestCase, self).setUp()
-        self.api = TestingAPI(None)
+        self.api = TestingAPI()
         mock.patch.object(FakeTransaction, 'commit').start()
 
     def test_transaction_nested(self):