bt2: update bindings to make test_component_class pass
authorSimon Marchi <simon.marchi@efficios.com>
Wed, 1 May 2019 15:30:28 +0000 (11:30 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 3 May 2019 22:19:40 +0000 (18:19 -0400)
This patch updates the binding with the goal of making
test_component_class pass.

Besides the obviously needed changes to the component class code, there
are a bunch of changes to things around it (component, graph), just
enough to make the test run.

One notable change is the _SharedObject._release method, used to make the
Python object stop manage the reference it currently owns.

Another functional change is how the component classes _query method
reports that it's not implemented.  Currently, it default to return the
NotImplemented object.  Instead, I made it raise a NotImplementedError.
Returning NotImplemented is useful if the caller has an alternative to
try when the method is not implemented.  In our case, there is nothing
to do if the specific class doesn't implement _query.  The exception is
caught in bt_py3_component_class_query, where other types of exceptions
need to be handled anyway.

Some changes are required to the library, to be able to get a specific
component class type from a specific component (e.g. a
bt_component_class_source from a bt_component_source).

Change-Id: I895817bc996c21b6a9c6d57605cc64f9ff61dfcc
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1083
Tested-by: jenkins
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
bindings/python/bt2/bt2/component.py
bindings/python/bt2/bt2/graph.py
bindings/python/bt2/bt2/native_bt_query_exec.i
bindings/python/bt2/bt2/object.py
bindings/python/bt2/bt2/query_executor.py
tests/bindings/python/bt2/test_component_class.py

index 82558ebd27372a5741ae3172e49970b9d7f214ef..459ae955b4572300474a6120a74936519233a36d 100644 (file)
@@ -39,20 +39,32 @@ _NO_PRINT_TRACEBACK = _env_var == '1'
 # have been created by Python code, but since we only have the pointer,
 # we can only wrap it in a generic way and lose the original Python
 # class.
+#
+# Subclasses must implement some methods that this base class uses:
+#
+#   - _as_component_class_ptr: static method, convert the passed component class
+#     pointer to a 'bt_component_class *'.
+
 class _GenericComponentClass(object._SharedObject):
     @property
     def name(self):
-        name = native_bt.component_class_get_name(self._ptr)
-        assert(name is not None)
+        ptr = self._as_component_class_ptr(self._ptr)
+        name = native_bt.component_class_get_name(ptr)
+        assert name is not None
         return name
 
     @property
     def description(self):
-        return native_bt.component_class_get_description(self._ptr)
+        ptr = self._as_component_class_ptr(self._ptr)
+        return native_bt.component_class_get_description(ptr)
 
     @property
     def help(self):
-        return native_bt.component_class_get_help(self._ptr)
+        ptr = self._as_component_class_ptr(self._ptr)
+        return native_bt.component_class_get_help(ptr)
+
+    def _component_class_ptr(self):
+        return self._as_component_class_ptr(self._ptr)
 
     def __eq__(self, other):
         if not isinstance(other, _GenericComponentClass):
@@ -66,15 +78,21 @@ class _GenericComponentClass(object._SharedObject):
 
 
 class _GenericSourceComponentClass(_GenericComponentClass):
-    pass
+    _get_ref = native_bt.component_class_source_get_ref
+    _put_ref = native_bt.component_class_source_put_ref
+    _as_component_class_ptr = native_bt.component_class_source_as_component_class
 
 
 class _GenericFilterComponentClass(_GenericComponentClass):
-    pass
+    _get_ref = native_bt.component_class_filter_get_ref
+    _put_ref = native_bt.component_class_filter_put_ref
+    _as_component_class_ptr = native_bt.component_class_filter_as_component_class
 
 
 class _GenericSinkComponentClass(_GenericComponentClass):
-    pass
+    _get_ref = native_bt.component_class_sink_get_ref
+    _put_ref = native_bt.component_class_sink_put_ref
+    _as_component_class_ptr = native_bt.component_class_sink_as_component_class
 
 
 def _handle_component_status(status, gen_error_msg):
@@ -157,9 +175,16 @@ class _ComponentPorts(collections.abc.Mapping):
 
 
 # This class holds the methods which are common to both generic
-# component objects and Python user component objects. They use the
-# internal native _ptr, however it was set, to call native API
-# functions.
+# component objects and Python user component objects.
+#
+# Subclasses must provide these methods or property:
+#
+#   - _borrow_component_class_ptr: static method, must return a pointer to the
+#     specialized component class (e.g. 'bt_component_class_sink *') of the
+#     passed specialized component pointer (e.g. 'bt_component_sink *').
+#   - _comp_cls_type: property, one of the native_bt.COMPONENT_CLASS_TYPE_*
+#     constants.
+
 class _Component:
     @property
     def name(self):
@@ -175,9 +200,9 @@ class _Component:
 
     @property
     def component_class(self):
-        cc_ptr = native_bt.component_get_class(self._ptr)
-        assert(cc_ptr)
-        return _create_generic_component_class_from_ptr(cc_ptr)
+        cc_ptr = self._borrow_component_class_ptr(self._ptr)
+        assert cc_ptr is not None
+        return _create_component_class_from_ptr_and_get_ref(cc_ptr, self._comp_cls_type)
 
     def __eq__(self, other):
         if not hasattr(other, 'addr'):
@@ -187,15 +212,21 @@ class _Component:
 
 
 class _SourceComponent(_Component):
-    pass
+    _borrow_component_class_ptr = native_bt.component_source_borrow_class_const
+    _comp_cls_type = native_bt.COMPONENT_CLASS_TYPE_SOURCE
+    _as_component_class_ptr = native_bt.component_class_source_as_component_class
 
 
 class _FilterComponent(_Component):
-    pass
+    _borrow_component_class_ptr = native_bt.component_filter_borrow_class_const
+    _comp_cls_type = native_bt.COMPONENT_CLASS_TYPE_FILTER
+    _as_component_class_ptr = native_bt.component_class_filter_as_component_class
 
 
 class _SinkComponent(_Component):
-    pass
+    _borrow_component_class_ptr = native_bt.component_sink_borrow_class_const
+    _comp_cls_type = native_bt.COMPONENT_CLASS_TYPE_SINK
+    _as_component_class_ptr = native_bt.component_class_sink_as_component_class
 
 
 # This is analogous to _GenericSourceComponentClass, but for source
@@ -230,6 +261,9 @@ class _GenericFilterComponent(object._SharedObject, _FilterComponent):
 # This is analogous to _GenericSinkComponentClass, but for sink
 # component objects.
 class _GenericSinkComponent(object._SharedObject, _SinkComponent):
+    _get_ref = native_bt.component_sink_get_ref
+    _put_ref = native_bt.component_sink_put_ref
+
     @property
     def input_ports(self):
         return _ComponentPorts(False, self,
@@ -252,14 +286,23 @@ _COMP_CLS_TYPE_TO_GENERIC_COMP_CLS_PYCLS = {
 }
 
 
-def _create_generic_component_from_ptr(ptr):
-    comp_cls_type = native_bt.component_get_class_type(ptr)
+# Create a component Python object of type _GenericSourceComponent,
+# _GenericFilterComponent or _GenericSinkComponent, depending on
+# comp_cls_type.
+#
+#    Steals the reference to ptr from the caller.
+
+def _create_component_from_ptr(ptr, comp_cls_type):
     return _COMP_CLS_TYPE_TO_GENERIC_COMP_PYCLS[comp_cls_type]._create_from_ptr(ptr)
 
+# Create a component class Python object of type
+# _GenericSourceComponentClass, _GenericFilterComponentClass or
+# _GenericSinkComponentClass, depending on comp_cls_type.
+#
+# Acquires a new reference to ptr.
 
-def _create_generic_component_class_from_ptr(ptr):
-    comp_cls_type = native_bt.component_class_get_type(ptr)
-    return _COMP_CLS_TYPE_TO_GENERIC_COMP_CLS_PYCLS[comp_cls_type]._create_from_ptr(ptr)
+def _create_component_class_from_ptr_and_get_ref(ptr, comp_cls_type):
+    return _COMP_CLS_TYPE_TO_GENERIC_COMP_CLS_PYCLS[comp_cls_type]._create_from_ptr_and_get_ref(ptr)
 
 
 def _trim_docstring(docstring):
@@ -422,13 +465,12 @@ class _UserComponentType(type):
         # create instance, not user-initialized yet
         self = cls.__new__(cls)
 
-        # pointer to native private component object (weak/borrowed)
+        # pointer to native self component object (weak/borrowed)
         self._ptr = comp_ptr
 
         # call user's __init__() method
         if params_ptr is not None:
-            native_bt.get(params_ptr)
-            params = bt2.value._create_from_ptr(params_ptr)
+            params = bt2.value._create_from_ptr_and_get_ref(params_ptr)
         else:
             params = None
 
@@ -453,15 +495,18 @@ class _UserComponentType(type):
 
     @property
     def name(cls):
-        return native_bt.component_class_get_name(cls._cc_ptr)
+        ptr = cls._as_component_class_ptr(cls._cc_ptr)
+        return native_bt.component_class_get_name(ptr)
 
     @property
     def description(cls):
-        return native_bt.component_class_get_description(cls._cc_ptr)
+        ptr = cls._as_component_class_ptr(cls._cc_ptr)
+        return native_bt.component_class_get_description(ptr)
 
     @property
     def help(cls):
-        return native_bt.component_class_get_help(cls._cc_ptr)
+        ptr = cls._as_component_class_ptr(cls._cc_ptr)
+        return native_bt.component_class_get_help(ptr)
 
     @property
     def addr(cls):
@@ -471,20 +516,16 @@ class _UserComponentType(type):
         # this can raise, in which case the native call to
         # bt_component_class_query() returns NULL
         if params_ptr is not None:
-            native_bt.get(params_ptr)
-            params = bt2.value._create_from_ptr(params_ptr)
+            params = bt2.value._create_from_ptr_and_get_ref(params_ptr)
         else:
             params = None
 
-        native_bt.get(query_exec_ptr)
-        query_exec = bt2.QueryExecutor._create_from_ptr(query_exec_ptr)
+        query_exec = bt2.QueryExecutor._create_from_ptr_and_get_ref(
+            query_exec_ptr)
 
         # this can raise, but the native side checks the exception
         results = cls._query(query_exec, obj, params)
 
-        if results is NotImplemented:
-            return results
-
         # this can raise, but the native side checks the exception
         results = bt2.create_value(results)
 
@@ -492,24 +533,20 @@ class _UserComponentType(type):
             results_addr = int(native_bt.value_null)
         else:
             # return new reference
-            results._get()
-            results_addr = int(results._ptr)
+            results_addr = int(results._release())
 
         return results_addr
 
     def _query(cls, query_executor, obj, params):
-        # BT catches this and returns NULL to the user
-        return NotImplemented
+        raise NotImplementedError
 
-    def __eq__(self, other):
-        if not hasattr(other, 'addr'):
-            return False
-
-        return self.addr == other.addr
+    def _component_class_ptr(self):
+        return self._as_component_class_ptr(self._cc_ptr)
 
     def __del__(cls):
         if hasattr(cls, '_cc_ptr'):
-            native_bt.put(cls._cc_ptr)
+            cc_ptr = cls._as_component_class_ptr(cls._cc_ptr)
+            native_bt.component_class_put_ref(cc_ptr)
 
 
 class _UserComponent(metaclass=_UserComponentType):
index 200f60214a489e1f8c88da3242fc6981fb0ca41c..e42b242568e4e2215a5976bb09636e7ba00587ff 100644 (file)
@@ -87,6 +87,9 @@ def _graph_ports_disconnected_listener_from_native(user_listener,
 
 
 class Graph(object._SharedObject):
+    _get_ref = native_bt.graph_get_ref
+    _put_ref = native_bt.graph_put_ref
+
     def __init__(self):
         ptr = native_bt.graph_create()
 
@@ -104,34 +107,26 @@ class Graph(object._SharedObject):
             raise bt2.Stop
         elif status == native_bt.GRAPH_STATUS_AGAIN:
             raise bt2.TryAgain
-        elif status == native_bt.GRAPH_STATUS_NO_SINK:
-            raise bt2.NoSinkComponent
-        elif status == native_bt.GRAPH_STATUS_CANNOT_CONSUME:
-            raise bt2.CannotConsumeGraph
         elif status < 0:
             raise bt2.Error(gen_error_msg)
 
-    def add_component(self, component_class, name, params=None):
-        if isinstance(component_class, bt2.component._GenericComponentClass):
-            cc_ptr = component_class._ptr
-        elif issubclass(component_class, bt2.component._UserComponent):
+    def add_sink_component(self, component_class, name, params=None):
+        if issubclass(component_class, bt2.component._UserSinkComponent):
             cc_ptr = component_class._cc_ptr
         else:
-            raise TypeError("'{}' is not a component class".format(component_class.__class__.__name__))
+            raise TypeError("'{}' is not a sink component class".format(
+                component_class.__class__.__name__))
 
         utils._check_str(name)
         params = bt2.create_value(params)
 
-        if params is None:
-            params_ptr = None
-        else:
-            params_ptr = params._ptr
+        params_ptr = params._ptr if params is not None else None
 
-        status, comp_ptr = native_bt.graph_add_component(self._ptr, cc_ptr,
-                                                         name, params_ptr)
-        self._handle_status(status, 'cannot add component to graph')
+        status, comp_ptr = native_bt.graph_add_sink_component(self._ptr, cc_ptr,
+                                                              name, params_ptr)
+        self._handle_status(status, 'cannot add sink component to graph')
         assert(comp_ptr)
-        return bt2.component._create_generic_component_from_ptr(comp_ptr)
+        return bt2.component._create_component_from_ptr(comp_ptr, native_bt.COMPONENT_CLASS_TYPE_SINK)
 
     def connect_ports(self, upstream_port, downstream_port):
         utils._check_type(upstream_port, bt2.port._OutputPort)
index 2f02ca81549078074174747509c01914743b0878..af78efaf1f5c0ad52b9ea3168cc2e036a2c6697f 100644 (file)
@@ -55,7 +55,7 @@ bt_query_executor_status bt_query_executor_query(
                bt_query_executor *query_executor,
                const bt_component_class *component_class,
                const char *object, const bt_value *params,
-               const bt_value **OUT_VALUE);
+               const bt_value **OUT);
 
 extern
 bt_query_executor_status bt_query_executor_cancel(
index 05c6deb75b63ac345aeabaa072ae63f836d83de5..ac5107d1eead943f44e8c42a41ce45d4dd29eb85 100644 (file)
@@ -123,5 +123,12 @@ class _SharedObject(_BaseObject):
         cls._get_ref(obj._ptr)
         return obj
 
+    def _release(self):
+        """Return the wrapped pointer, transfer its ownership to the
+        caller."""
+        ptr = self._ptr
+        self._ptr = None
+        return ptr
+
     def __del__(self):
         self._put_ref(self._ptr)
index db7db61028d73a9cce32ba19bdab2fec5b3b8663..f90bb902651cf5fb98c2b43bcab052c35068f250 100644 (file)
@@ -26,14 +26,17 @@ import bt2
 
 
 class QueryExecutor(object._SharedObject):
+    _get_ref = native_bt.query_executor_get_ref
+    _put_ref = native_bt.query_executor_put_ref
+
     def _handle_status(self, status, gen_error_msg):
-        if status == native_bt.QUERY_STATUS_AGAIN:
+        if status == native_bt.QUERY_EXECUTOR_STATUS_AGAIN:
             raise bt2.TryAgain
-        elif status == native_bt.QUERY_STATUS_EXECUTOR_CANCELED:
+        elif status == native_bt.QUERY_EXECUTOR_STATUS_CANCELED:
             raise bt2.QueryExecutorCanceled
-        elif status == native_bt.QUERY_STATUS_INVALID_OBJECT:
+        elif status == native_bt.QUERY_EXECUTOR_STATUS_INVALID_OBJECT:
             raise bt2.InvalidQueryObject
-        elif status == native_bt.QUERY_STATUS_INVALID_PARAMS:
+        elif status == native_bt.QUERY_EXECUTOR_STATUS_INVALID_PARAMS:
             raise bt2.InvalidQueryParams
         elif status < 0:
             raise bt2.Error(gen_error_msg)
@@ -78,10 +81,7 @@ class QueryExecutor(object._SharedObject):
             params = bt2.create_value(params)
             params_ptr = params._ptr
 
-        if isinstance(component_class, bt2.component._GenericComponentClass):
-            cc_ptr = component_class._ptr
-        else:
-            cc_ptr = component_class._cc_ptr
+        cc_ptr = component_class._component_class_ptr()
 
         status, result_ptr = native_bt.query_executor_query(self._ptr, cc_ptr,
                                                             object, params_ptr)
index a9cb9fe95d084548bcff3457fffd0fb2d87dafa8..1debdf7bba97ef2557b25249b8c58114a51454ff 100644 (file)
@@ -1,10 +1,8 @@
 from bt2 import value
 import unittest
-import copy
 import bt2
 
 
-@unittest.skip("this is broken")
 class UserComponentClassTestCase(unittest.TestCase):
     def _test_no_init(self, cls):
         with self.assertRaises(bt2.Error):
@@ -272,7 +270,6 @@ class UserComponentClassTestCase(unittest.TestCase):
         self.assertEqual(MySink, MySink)
 
 
-@unittest.skip("this is broken")
 class GenericComponentClassTestCase(unittest.TestCase):
     def setUp(self):
         class MySink(bt2._UserSinkComponent):
@@ -290,7 +287,7 @@ class GenericComponentClassTestCase(unittest.TestCase):
 
         self._py_comp_cls = MySink
         graph = bt2.Graph()
-        comp = graph.add_component(MySink, 'salut')
+        comp = graph.add_sink_component(MySink, 'salut')
         self._comp_cls = comp.component_class
         self.assertTrue(issubclass(type(self._comp_cls),
                                    bt2.component._GenericComponentClass))
This page took 0.031049 seconds and 4 git commands to generate.