From bbb3650ffa25676b8425802ba1dd5614fc1db17f Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 1 May 2019 11:30:28 -0400 Subject: [PATCH] bt2: update bindings to make test_component_class pass 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1083 Tested-by: jenkins Reviewed-by: Philippe Proulx --- bindings/python/bt2/bt2/component.py | 125 ++++++++++++------ bindings/python/bt2/bt2/graph.py | 29 ++-- .../python/bt2/bt2/native_bt_query_exec.i | 2 +- bindings/python/bt2/bt2/object.py | 7 + bindings/python/bt2/bt2/query_executor.py | 16 +-- .../python/bt2/test_component_class.py | 5 +- 6 files changed, 110 insertions(+), 74 deletions(-) diff --git a/bindings/python/bt2/bt2/component.py b/bindings/python/bt2/bt2/component.py index 82558ebd..459ae955 100644 --- a/bindings/python/bt2/bt2/component.py +++ b/bindings/python/bt2/bt2/component.py @@ -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): diff --git a/bindings/python/bt2/bt2/graph.py b/bindings/python/bt2/bt2/graph.py index 200f6021..e42b2425 100644 --- a/bindings/python/bt2/bt2/graph.py +++ b/bindings/python/bt2/bt2/graph.py @@ -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) diff --git a/bindings/python/bt2/bt2/native_bt_query_exec.i b/bindings/python/bt2/bt2/native_bt_query_exec.i index 2f02ca81..af78efaf 100644 --- a/bindings/python/bt2/bt2/native_bt_query_exec.i +++ b/bindings/python/bt2/bt2/native_bt_query_exec.i @@ -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( diff --git a/bindings/python/bt2/bt2/object.py b/bindings/python/bt2/bt2/object.py index 05c6deb7..ac5107d1 100644 --- a/bindings/python/bt2/bt2/object.py +++ b/bindings/python/bt2/bt2/object.py @@ -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) diff --git a/bindings/python/bt2/bt2/query_executor.py b/bindings/python/bt2/bt2/query_executor.py index db7db610..f90bb902 100644 --- a/bindings/python/bt2/bt2/query_executor.py +++ b/bindings/python/bt2/bt2/query_executor.py @@ -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) diff --git a/tests/bindings/python/bt2/test_component_class.py b/tests/bindings/python/bt2/test_component_class.py index a9cb9fe9..1debdf7b 100644 --- a/tests/bindings/python/bt2/test_component_class.py +++ b/tests/bindings/python/bt2/test_component_class.py @@ -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)) -- 2.34.1