From 76b6c2f71a485f0cc9b766e1a54b9f7330ccd907 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 24 Jul 2019 12:11:01 -0400 Subject: [PATCH] lib: rename INVALID_OBJECT status to UNKNOWN_OBJECT Rename INVALID_OBJECT to UNKNOWN_OBJECT, because it represents better the situation: the passed object is not necessarily invalid (any string is a valid object), it's just that the component class doesn't know that object. Change its numerical value to something positive, because it should not be considered as an error (which negative numerical values represent). A function returning UNKNOWN_OBJECT should not append an error cause. Also, in the Python bindings, change the default implementation of _query to raise bt2.UnknownObject instead of NotImplementedError. NotImplementedError would result in an ERROR status code, whereas bt2.UnknownObject becomes UNKNOWN_OBJECT. This matches the behavior of a component class implemented in C, where if it doesn't provide a query method, it will automatically return UNKNOWN_OBJECT. Change-Id: Ica1418272b5bf2bbe8e96d639c16f56191151d79 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/1760 CI-Build: Philippe Proulx Tested-by: jenkins Reviewed-by: Philippe Proulx --- include/babeltrace2/babeltrace.h | 2 +- include/babeltrace2/func-status.h | 9 +++++---- include/babeltrace2/graph/component-class.h | 2 +- include/babeltrace2/graph/query-executor.h | 2 +- src/bindings/python/bt2/bt2/__init__.py | 7 ++++++- src/bindings/python/bt2/bt2/component.py | 2 +- .../python/bt2/bt2/native_bt_component_class.i | 14 +++++++------- src/bindings/python/bt2/bt2/utils.py | 6 +++--- src/cli/babeltrace2.c | 4 ++-- src/common/common.h | 4 ++-- src/lib/func-status.h | 2 +- src/lib/graph/query-executor.c | 2 +- src/plugins/ctf/fs-src/fs.c | 2 +- src/plugins/ctf/lttng-live/lttng-live.c | 2 +- tests/bindings/python/bt2/test_component_class.py | 2 +- tests/bindings/python/bt2/test_query_executor.py | 6 +++--- .../cli/auto-source-discovery/bt_plugin_test.py | 6 +++--- 17 files changed, 40 insertions(+), 34 deletions(-) diff --git a/include/babeltrace2/babeltrace.h b/include/babeltrace2/babeltrace.h index 3b7bb753..d4613ca4 100644 --- a/include/babeltrace2/babeltrace.h +++ b/include/babeltrace2/babeltrace.h @@ -161,7 +161,7 @@ #undef __BT_FUNC_STATUS_END #undef __BT_FUNC_STATUS_ERROR #undef __BT_FUNC_STATUS_INTERRUPTED -#undef __BT_FUNC_STATUS_INVALID_OBJECT +#undef __BT_FUNC_STATUS_UNKNOWN_OBJECT #undef __BT_FUNC_STATUS_MEMORY_ERROR #undef __BT_FUNC_STATUS_NOT_FOUND #undef __BT_FUNC_STATUS_OK diff --git a/include/babeltrace2/func-status.h b/include/babeltrace2/func-status.h index 4f8e61a9..75cee213 100644 --- a/include/babeltrace2/func-status.h +++ b/include/babeltrace2/func-status.h @@ -41,10 +41,6 @@ # define __BT_FUNC_STATUS_OVERFLOW_ERROR -75 #endif -/* Invalid query object */ -#ifndef __BT_FUNC_STATUS_INVALID_OBJECT -# define __BT_FUNC_STATUS_INVALID_OBJECT -23 -#endif /* Memory allocation error */ #ifndef __BT_FUNC_STATUS_MEMORY_ERROR @@ -80,3 +76,8 @@ #ifndef __BT_FUNC_STATUS_AGAIN # define __BT_FUNC_STATUS_AGAIN 11 #endif + +/* Unknown query object */ +#ifndef __BT_FUNC_STATUS_UNKNOWN_OBJECT +# define __BT_FUNC_STATUS_UNKNOWN_OBJECT 42 +#endif diff --git a/include/babeltrace2/graph/component-class.h b/include/babeltrace2/graph/component-class.h index 8a35146c..deba3d03 100644 --- a/include/babeltrace2/graph/component-class.h +++ b/include/babeltrace2/graph/component-class.h @@ -50,7 +50,7 @@ typedef enum bt_component_class_query_method_status { BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_AGAIN = __BT_FUNC_STATUS_AGAIN, BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_ERROR = __BT_FUNC_STATUS_ERROR, BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, - BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_INVALID_OBJECT = __BT_FUNC_STATUS_INVALID_OBJECT, + BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_UNKNOWN_OBJECT = __BT_FUNC_STATUS_UNKNOWN_OBJECT, } bt_component_class_query_method_status; typedef enum bt_component_class_message_iterator_init_method_status { diff --git a/include/babeltrace2/graph/query-executor.h b/include/babeltrace2/graph/query-executor.h index cf6ec244..460899bd 100644 --- a/include/babeltrace2/graph/query-executor.h +++ b/include/babeltrace2/graph/query-executor.h @@ -42,7 +42,7 @@ typedef enum bt_query_executor_query_status { BT_QUERY_EXECUTOR_QUERY_STATUS_AGAIN = __BT_FUNC_STATUS_AGAIN, BT_QUERY_EXECUTOR_QUERY_STATUS_ERROR = __BT_FUNC_STATUS_ERROR, BT_QUERY_EXECUTOR_QUERY_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, - BT_QUERY_EXECUTOR_QUERY_STATUS_INVALID_OBJECT = __BT_FUNC_STATUS_INVALID_OBJECT, + BT_QUERY_EXECUTOR_QUERY_STATUS_UNKNOWN_OBJECT = __BT_FUNC_STATUS_UNKNOWN_OBJECT, } bt_query_executor_query_status; extern diff --git a/src/bindings/python/bt2/bt2/__init__.py b/src/bindings/python/bt2/bt2/__init__.py index 313ede9b..078ab0d0 100644 --- a/src/bindings/python/bt2/bt2/__init__.py +++ b/src/bindings/python/bt2/bt2/__init__.py @@ -160,7 +160,12 @@ class _MemoryError(_Error): '''Raised when an operation fails due to memory issues.''' -class InvalidObject(Exception): +class UnknownObject(Exception): + ''' + Raised when a component class handles a query for an object it doesn't + know about. + ''' + pass diff --git a/src/bindings/python/bt2/bt2/component.py b/src/bindings/python/bt2/bt2/component.py index 05ebe761..865ff135 100644 --- a/src/bindings/python/bt2/bt2/component.py +++ b/src/bindings/python/bt2/bt2/component.py @@ -603,7 +603,7 @@ class _UserComponentType(type): return int(results_ptr) def _user_query(cls, query_executor, obj, params, log_level): - raise NotImplementedError + raise bt2.UnknownObject def _bt_component_class_ptr(self): return self._bt_as_component_class_ptr(self._bt_cc_ptr) diff --git a/src/bindings/python/bt2/bt2/native_bt_component_class.i b/src/bindings/python/bt2/bt2/native_bt_component_class.i index e71e6bf8..90fc4aba 100644 --- a/src/bindings/python/bt2/bt2/native_bt_component_class.i +++ b/src/bindings/python/bt2/bt2/native_bt_component_class.i @@ -97,7 +97,7 @@ static PyObject *py_mod_bt2_exc_error_type = NULL; static PyObject *py_mod_bt2_exc_memory_error = NULL; static PyObject *py_mod_bt2_exc_try_again_type = NULL; static PyObject *py_mod_bt2_exc_stop_type = NULL; -static PyObject *py_mod_bt2_exc_invalid_object_type = NULL; +static PyObject *py_mod_bt2_exc_unknown_object_type = NULL; static void bt_bt2_cc_init_from_bt2(void) @@ -124,9 +124,9 @@ void bt_bt2_cc_init_from_bt2(void) py_mod_bt2_exc_stop_type = PyObject_GetAttrString(py_mod_bt2, "Stop"); BT_ASSERT(py_mod_bt2_exc_stop_type); - py_mod_bt2_exc_invalid_object_type = - PyObject_GetAttrString(py_mod_bt2, "InvalidObject"); - BT_ASSERT(py_mod_bt2_exc_invalid_object_type); + py_mod_bt2_exc_unknown_object_type = + PyObject_GetAttrString(py_mod_bt2, "UnknownObject"); + BT_ASSERT(py_mod_bt2_exc_unknown_object_type); } static @@ -149,7 +149,7 @@ void bt_bt2_cc_exit_handler(void) Py_XDECREF(py_mod_bt2_exc_error_type); Py_XDECREF(py_mod_bt2_exc_try_again_type); Py_XDECREF(py_mod_bt2_exc_stop_type); - Py_XDECREF(py_mod_bt2_exc_invalid_object_type); + Py_XDECREF(py_mod_bt2_exc_unknown_object_type); } @@ -388,8 +388,8 @@ int py_exc_to_status(bt_self_component_class *self_component_class, py_mod_bt2_exc_stop_type)) { status = __BT_FUNC_STATUS_END; } else if (PyErr_GivenExceptionMatches(exc, - py_mod_bt2_exc_invalid_object_type)) { - status = __BT_FUNC_STATUS_INVALID_OBJECT; + py_mod_bt2_exc_unknown_object_type)) { + status = __BT_FUNC_STATUS_UNKNOWN_OBJECT; } else { /* Unknown exception: convert to general error */ log_exception_and_maybe_append_error(BT_LOG_WARNING, true, diff --git a/src/bindings/python/bt2/bt2/utils.py b/src/bindings/python/bt2/bt2/utils.py index a3df357e..77ebe606 100644 --- a/src/bindings/python/bt2/bt2/utils.py +++ b/src/bindings/python/bt2/bt2/utils.py @@ -156,11 +156,11 @@ def _handle_func_status(status, msg=None): raise bt2._OverflowError else: raise bt2._OverflowError(msg) - elif status == native_bt.__BT_FUNC_STATUS_INVALID_OBJECT: + elif status == native_bt.__BT_FUNC_STATUS_UNKNOWN_OBJECT: if msg is None: - raise bt2.InvalidObject + raise bt2.UnknownObject else: - raise bt2.InvalidObject(msg) + raise bt2.UnknownObject(msg) else: assert False diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c index bafda3b5..8301fba8 100644 --- a/src/cli/babeltrace2.c +++ b/src/cli/babeltrace2.c @@ -173,8 +173,8 @@ int query(struct bt_config *cfg, const bt_component_class *comp_cls, } goto error; - case BT_QUERY_EXECUTOR_QUERY_STATUS_INVALID_OBJECT: - *fail_reason = "invalid or unknown query object"; + case BT_QUERY_EXECUTOR_QUERY_STATUS_UNKNOWN_OBJECT: + *fail_reason = "unknown query object"; goto error; case BT_QUERY_EXECUTOR_QUERY_STATUS_MEMORY_ERROR: *fail_reason = "not enough memory"; diff --git a/src/common/common.h b/src/common/common.h index 88de75e6..4a2362e3 100644 --- a/src/common/common.h +++ b/src/common/common.h @@ -603,8 +603,8 @@ const char *bt_common_func_status_string(int status) switch (status) { case __BT_FUNC_STATUS_OVERFLOW_ERROR: return "OVERFLOW"; - case __BT_FUNC_STATUS_INVALID_OBJECT: - return "INVALID_OBJECT"; + case __BT_FUNC_STATUS_UNKNOWN_OBJECT: + return "UNKNOWN_OBJECT"; case __BT_FUNC_STATUS_MEMORY_ERROR: return "MEMORY_ERROR"; case __BT_FUNC_STATUS_ERROR: diff --git a/src/lib/func-status.h b/src/lib/func-status.h index e5aca4aa..d373b3c4 100644 --- a/src/lib/func-status.h +++ b/src/lib/func-status.h @@ -34,7 +34,7 @@ #define BT_FUNC_STATUS_END __BT_FUNC_STATUS_END #define BT_FUNC_STATUS_ERROR __BT_FUNC_STATUS_ERROR #define BT_FUNC_STATUS_INTERRUPTED __BT_FUNC_STATUS_INTERRUPTED -#define BT_FUNC_STATUS_INVALID_OBJECT __BT_FUNC_STATUS_INVALID_OBJECT +#define BT_FUNC_STATUS_UNKNOWN_OBJECT __BT_FUNC_STATUS_UNKNOWN_OBJECT #define BT_FUNC_STATUS_MEMORY_ERROR __BT_FUNC_STATUS_MEMORY_ERROR #define BT_FUNC_STATUS_NOT_FOUND __BT_FUNC_STATUS_NOT_FOUND #define BT_FUNC_STATUS_OK __BT_FUNC_STATUS_OK diff --git a/src/lib/graph/query-executor.c b/src/lib/graph/query-executor.c index ce6dd6d9..a602911c 100644 --- a/src/lib/graph/query-executor.c +++ b/src/lib/graph/query-executor.c @@ -173,7 +173,7 @@ enum bt_query_executor_query_status bt_query_executor_query( /* Not an error: nothing to query */ BT_LIB_LOGD("Component class has no registered query method: " "%!+C", comp_cls); - status = BT_FUNC_STATUS_INVALID_OBJECT; + status = BT_FUNC_STATUS_UNKNOWN_OBJECT; goto end; } diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index e4eb3852..1ff98131 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/src/plugins/ctf/fs-src/fs.c @@ -2007,7 +2007,7 @@ bt_component_class_query_method_status ctf_fs_query( status = support_info_query(comp_class, params, log_level, result); } else { BT_LOGE("Unknown query object `%s`", object); - status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_INVALID_OBJECT; + status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_UNKNOWN_OBJECT; goto end; } end: diff --git a/src/plugins/ctf/lttng-live/lttng-live.c b/src/plugins/ctf/lttng-live/lttng-live.c index df5639b1..124b9d12 100644 --- a/src/plugins/ctf/lttng-live/lttng-live.c +++ b/src/plugins/ctf/lttng-live/lttng-live.c @@ -1456,7 +1456,7 @@ bt_component_class_query_method_status lttng_live_query( log_level); } else { BT_COMP_LOGI("Unknown query object `%s`", object); - status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_INVALID_OBJECT; + status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_UNKNOWN_OBJECT; goto end; } diff --git a/tests/bindings/python/bt2/test_component_class.py b/tests/bindings/python/bt2/test_component_class.py index fd36d240..aa13cd6a 100644 --- a/tests/bindings/python/bt2/test_component_class.py +++ b/tests/bindings/python/bt2/test_component_class.py @@ -184,7 +184,7 @@ class UserComponentClassTestCase(unittest.TestCase): def _user_consume(self): pass - with self.assertRaises(bt2._Error): + with self.assertRaises(bt2.UnknownObject): bt2.QueryExecutor().query(MySink, 'obj', 23) def test_query_raises(self): diff --git a/tests/bindings/python/bt2/test_query_executor.py b/tests/bindings/python/bt2/test_query_executor.py index e4f1fa5b..6d0cc3bd 100644 --- a/tests/bindings/python/bt2/test_query_executor.py +++ b/tests/bindings/python/bt2/test_query_executor.py @@ -96,16 +96,16 @@ class QueryExecutorTestCase(unittest.TestCase): self.assertEqual(cause.component_class_type, bt2.ComponentClassType.SINK) self.assertEqual(cause.component_class_name, 'MySink') - def test_query_invalid_object(self): + def test_query_unknown_object(self): class MySink(bt2._UserSinkComponent): def _user_consume(self): pass @classmethod def _user_query(cls, query_exec, obj, params, log_level): - raise bt2.InvalidObject + raise bt2.UnknownObject - with self.assertRaises(bt2.InvalidObject): + with self.assertRaises(bt2.UnknownObject): res = bt2.QueryExecutor().query(MySink, 'obj', [17, 23]) def test_query_logging_level_invalid_type(self): diff --git a/tests/data/cli/auto-source-discovery/bt_plugin_test.py b/tests/data/cli/auto-source-discovery/bt_plugin_test.py index 002e9c33..8c212a34 100644 --- a/tests/data/cli/auto-source-discovery/bt_plugin_test.py +++ b/tests/data/cli/auto-source-discovery/bt_plugin_test.py @@ -49,7 +49,7 @@ class TestSourceExt(Base, bt2._UserSourceComponent, message_iterator_class=TestI else: return 0 else: - raise bt2.InvalidObject + raise bt2.UnknownObject @bt2.plugin_component_class @@ -72,7 +72,7 @@ class TestSourceSomeDir( else: return 0 else: - raise bt2.InvalidObject + raise bt2.UnknownObject @bt2.plugin_component_class @@ -91,7 +91,7 @@ class TestSourceABCDE(Base, bt2._UserSourceComponent, message_iterator_class=Tes else 0.0 ) else: - raise bt2.InvalidObject + raise bt2.UnknownObject class TestSourceNoQuery(bt2._UserSourceComponent, message_iterator_class=TestIter): -- 2.34.1