From 51f034157576cee149f78e2d5d555c5c0769bdf6 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 13:14:37 -0500 Subject: [PATCH 01/14] Build, do not import dynamically generated classes Signed-off-by: mulhern --- pyproject.toml | 3 ++ src/stratis_cli/_actions/_dynamic.py | 60 ++++++++++++++++++++++++++++ src/stratis_cli/_actions/_top.py | 17 ++++---- 3 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 src/stratis_cli/_actions/_dynamic.py diff --git a/pyproject.toml b/pyproject.toml index fed528d4a..6b1808967 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,6 @@ [build-system] requires = ["setuptools"] build-backend = "setuptools.build_meta" + +[tool.pylint] +good-names="Manager,ObjectManager,Report" diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py new file mode 100644 index 000000000..a289229bc --- /dev/null +++ b/src/stratis_cli/_actions/_dynamic.py @@ -0,0 +1,60 @@ +# Copyright 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Dynamic class generation +""" +# isort: STDLIB +import os +import xml.etree.ElementTree as ET # nosec B405 +from enum import Enum + +# isort: FIRSTPARTY +from dbus_python_client_gen import DPClientGenerationError, make_class + +from .._errors import StratisCliGenerationError +from ._constants import MANAGER_INTERFACE, REPORT_INTERFACE +from ._environment import get_timeout +from ._introspect import SPECS + +DBUS_TIMEOUT_SECONDS = 120 + +TIMEOUT = get_timeout( + os.environ.get("STRATIS_DBUS_TIMEOUT", DBUS_TIMEOUT_SECONDS * 1000) +) + + +class ClassKey(Enum): + """ + Keys for dynamically generated classes. + """ + + MANAGER = ("Manager", MANAGER_INTERFACE) + OBJECT_MANAGER = ("ObjectManager", "org.freedesktop.DBus.ObjectManager") + REPORT = ("Report", REPORT_INTERFACE) + + +def make_dyn_class(key): + """ + Dynamically generate a class from introspection specification. + """ + try: + return make_class( + key.value[0], + ET.fromstring(SPECS[key.value[1]]), # nosec B314 + TIMEOUT, + ) + except DPClientGenerationError as err: # pragma: no cover + raise StratisCliGenerationError( + "Failed to generate some class needed for invoking dbus-python methods" + ) from err diff --git a/src/stratis_cli/_actions/_top.py b/src/stratis_cli/_actions/_top.py index 7f2c79944..d8195dc60 100644 --- a/src/stratis_cli/_actions/_top.py +++ b/src/stratis_cli/_actions/_top.py @@ -33,6 +33,7 @@ from .._stratisd_constants import ReportKey, StratisdErrors from ._connection import get_object from ._constants import TOP_OBJECT +from ._dynamic import ClassKey, make_dyn_class from ._formatting import print_table @@ -44,8 +45,7 @@ def _fetch_keylist(proxy): :rtype: list of str :raises StratisCliEngineError: """ - # pylint: disable=import-outside-toplevel - from ._data import Manager + Manager = make_dyn_class(ClassKey.MANAGER) (keys, return_code, message) = Manager.Methods.ListKeys(proxy, {}) if return_code != StratisdErrors.OK: # pragma: no cover @@ -68,8 +68,7 @@ def _add_update_key(proxy, key_desc, capture_key, *, keyfile_path): """ assert capture_key == (keyfile_path is None) - # pylint: disable=import-outside-toplevel - from ._data import Manager + Manager = make_dyn_class(ClassKey.MANAGER) if capture_key: password = getpass(prompt="Enter key data followed by the return key: ") @@ -117,9 +116,8 @@ def get_report(namespace): :raises StratisCliEngineError: """ - # pylint: disable=import-outside-toplevel if namespace.report_name == ReportKey.MANAGED_OBJECTS.value: - from ._data import ObjectManager + ObjectManager = make_dyn_class(ClassKey.OBJECT_MANAGER) json_report = ObjectManager.Methods.GetManagedObjects( get_object(TOP_OBJECT), {} @@ -127,14 +125,14 @@ def get_report(namespace): else: if namespace.report_name == ReportKey.ENGINE_STATE.value: - from ._data import Manager + Manager = make_dyn_class(ClassKey.MANAGER) (report, return_code, message) = Manager.Methods.EngineStateReport( get_object(TOP_OBJECT), {} ) else: - from ._data import Report + Report = make_dyn_class(ClassKey.REPORT) (report, return_code, message) = Report.Methods.GetReport( get_object(TOP_OBJECT), {"name": namespace.report_name} @@ -242,8 +240,7 @@ def unset_key(namespace): :raises StratisCliNoChangeError: :raises StratisCliIncoherenceError: """ - # pylint: disable=import-outside-toplevel - from ._data import Manager + Manager = make_dyn_class(ClassKey.MANAGER) proxy = get_object(TOP_OBJECT) From 02ca9b0d1b4ba95d652da8c8dbb40c536ed96470 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 18:29:18 -0500 Subject: [PATCH 02/14] comment --- src/stratis_cli/_actions/_dynamic.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index a289229bc..9fa5d6aa6 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -47,6 +47,8 @@ class ClassKey(Enum): def make_dyn_class(key): """ Dynamically generate a class from introspection specification. + + :param ClassKey key: key that identifies the class to make """ try: return make_class( From 7a93fbc4e79f053f9d9649604b54f13de207b558 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 18:35:32 -0500 Subject: [PATCH 03/14] Use another class for class generation Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 9fa5d6aa6..c6ab1ac6f 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -34,14 +34,27 @@ ) +class ClassInfo: # pylint: disable=too-few-public-methods + """ + Information used to construct the dynamically generated class. + """ + + def __init__(self, name, interface_name): + """ + Initializer. + """ + self.name = name + self.interface_name = interface_name + + class ClassKey(Enum): """ Keys for dynamically generated classes. """ - MANAGER = ("Manager", MANAGER_INTERFACE) - OBJECT_MANAGER = ("ObjectManager", "org.freedesktop.DBus.ObjectManager") - REPORT = ("Report", REPORT_INTERFACE) + MANAGER = ClassInfo("Manager", MANAGER_INTERFACE) + OBJECT_MANAGER = ClassInfo("ObjectManager", "org.freedesktop.DBus.ObjectManager") + REPORT = ClassInfo("Report", REPORT_INTERFACE) def make_dyn_class(key): @@ -52,11 +65,12 @@ def make_dyn_class(key): """ try: return make_class( - key.value[0], - ET.fromstring(SPECS[key.value[1]]), # nosec B314 + key.value.name, + ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 TIMEOUT, ) except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( - "Failed to generate some class needed for invoking dbus-python methods" + f"Failed to generate class {key.value.name} needed for invoking " + "dbus-python methods" ) from err From c4d4c502342ec8563283f7a80477f5e3a2890733 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 18:52:56 -0500 Subject: [PATCH 04/14] Add method modifications Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 42 +++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index c6ab1ac6f..b1f4ad130 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -57,6 +57,31 @@ class ClassKey(Enum): REPORT = ClassInfo("Report", REPORT_INTERFACE) +def _add_abs_path_assertion(klass, method_name, key): + """ + Set method_name of method_klass to a new method which checks that the + device paths values at key are absolute paths. + + :param klass: the klass to which this metthod belongs + :param str method_name: the name of the method + :param str key: the key at which the paths can be found in the arguments + """ + method_class = getattr(klass, "Methods") + orig_method = getattr(method_class, method_name) + + def new_method(proxy, args): + """ + New path method + """ + rel_paths = [path for path in args[key] if not os.path.isabs(path)] + assert ( + rel_paths == [] + ), f"Precondition violated: paths {', '.join(rel_paths)} should be absolute" + return orig_method(proxy, args) + + setattr(method_class, method_name, new_method) + + def make_dyn_class(key): """ Dynamically generate a class from introspection specification. @@ -64,11 +89,26 @@ def make_dyn_class(key): :param ClassKey key: key that identifies the class to make """ try: - return make_class( + klass = make_class( key.value.name, ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 TIMEOUT, ) + + try: + if key == ClassKey.MANAGER: + _add_abs_path_assertion(klass, "CreatePool", "devices") + except AttributeError as err: # pragma: no cover + # This can only happen if the expected method is missing from + # the XML spec or code generation has a bug, we will never + # test for these conditions. + raise StratisCliGenerationError( + "Malformed class definition; could not access a class or " + "method in the generated class definition" + ) from err + + return klass + except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( f"Failed to generate class {key.value.name} needed for invoking " From 398ae7c9831db1ce6775d1cd58c556f05323c6c3 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 20:02:15 -0500 Subject: [PATCH 05/14] More things Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 39 ++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index b1f4ad130..71b509f74 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -23,7 +23,12 @@ from dbus_python_client_gen import DPClientGenerationError, make_class from .._errors import StratisCliGenerationError -from ._constants import MANAGER_INTERFACE, REPORT_INTERFACE +from ._constants import ( + FILESYSTEM_INTERFACE, + MANAGER_INTERFACE, + POOL_INTERFACE, + REPORT_INTERFACE, +) from ._environment import get_timeout from ._introspect import SPECS @@ -39,12 +44,12 @@ class ClassInfo: # pylint: disable=too-few-public-methods Information used to construct the dynamically generated class. """ - def __init__(self, name, interface_name): + def __init__(self, interface_name, *, invoke_name=None): """ Initializer. """ - self.name = name self.interface_name = interface_name + self.invoke_name = invoke_name class ClassKey(Enum): @@ -52,9 +57,23 @@ class ClassKey(Enum): Keys for dynamically generated classes. """ - MANAGER = ClassInfo("Manager", MANAGER_INTERFACE) - OBJECT_MANAGER = ClassInfo("ObjectManager", "org.freedesktop.DBus.ObjectManager") - REPORT = ClassInfo("Report", REPORT_INTERFACE) + FILESYSTEM = ClassInfo(FILESYSTEM_INTERFACE, invoke_name="Filesystem") + MANAGER = ClassInfo(MANAGER_INTERFACE, invoke_name="Manager") + OBJECT_MANAGER = ClassInfo( + "org.freedesktop.DBus.ObjectManager", invoke_name="ObjectManager" + ) + POOL = ClassInfo(POOL_INTERFACE, invoke_name="Pool") + REPORT = ClassInfo(REPORT_INTERFACE, invoke_name="Report") + + +class PurposeKey(Enum): + """ + Purpose of class to be created. + """ + + INVOKE = 0 # invoke D-Bus methods + OBJECT = 1 # represent object in GetManagedObjects result + SEARCH = 2 # search for object in GEtManagedObjects result def _add_abs_path_assertion(klass, method_name, key): @@ -90,7 +109,7 @@ def make_dyn_class(key): """ try: klass = make_class( - key.value.name, + key.value.invoke_name, ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 TIMEOUT, ) @@ -98,6 +117,10 @@ def make_dyn_class(key): try: if key == ClassKey.MANAGER: _add_abs_path_assertion(klass, "CreatePool", "devices") + if key == ClassKey.POOL: + _add_abs_path_assertion(klass, "InitCache", "devices") + _add_abs_path_assertion(klass, "AddCacheDevs", "devices") + _add_abs_path_assertion(klass, "AddDataDevs", "devices") except AttributeError as err: # pragma: no cover # This can only happen if the expected method is missing from # the XML spec or code generation has a bug, we will never @@ -111,6 +134,6 @@ def make_dyn_class(key): except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( - f"Failed to generate class {key.value.name} needed for invoking " + f"Failed to generate class {key.value.invoke_name} needed for invoking " "dbus-python methods" ) from err From 13cc0c33237731415bd2dd1d8930b223a4cebf95 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 20:06:20 -0500 Subject: [PATCH 06/14] Use purpose argument Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 59 +++++++++++++++------------- src/stratis_cli/_actions/_top.py | 14 +++---- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 71b509f74..96856ae24 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -66,7 +66,7 @@ class ClassKey(Enum): REPORT = ClassInfo(REPORT_INTERFACE, invoke_name="Report") -class PurposeKey(Enum): +class Purpose(Enum): """ Purpose of class to be created. """ @@ -101,39 +101,42 @@ def new_method(proxy, args): setattr(method_class, method_name, new_method) -def make_dyn_class(key): +def make_dyn_class(key, purpose): """ Dynamically generate a class from introspection specification. :param ClassKey key: key that identifies the class to make """ - try: - klass = make_class( - key.value.invoke_name, - ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 - TIMEOUT, - ) - + if purpose is Purpose.INVOKE: try: - if key == ClassKey.MANAGER: - _add_abs_path_assertion(klass, "CreatePool", "devices") - if key == ClassKey.POOL: - _add_abs_path_assertion(klass, "InitCache", "devices") - _add_abs_path_assertion(klass, "AddCacheDevs", "devices") - _add_abs_path_assertion(klass, "AddDataDevs", "devices") - except AttributeError as err: # pragma: no cover - # This can only happen if the expected method is missing from - # the XML spec or code generation has a bug, we will never - # test for these conditions. + klass = make_class( + key.value.invoke_name, + ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 + TIMEOUT, + ) + + try: + if key == ClassKey.MANAGER: + _add_abs_path_assertion(klass, "CreatePool", "devices") + if key == ClassKey.POOL: + _add_abs_path_assertion(klass, "InitCache", "devices") + _add_abs_path_assertion(klass, "AddCacheDevs", "devices") + _add_abs_path_assertion(klass, "AddDataDevs", "devices") + except AttributeError as err: # pragma: no cover + # This can only happen if the expected method is missing from + # the XML spec or code generation has a bug, we will never + # test for these conditions. + raise StratisCliGenerationError( + "Malformed class definition; could not access a class or " + "method in the generated class definition" + ) from err + + return klass + + except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( - "Malformed class definition; could not access a class or " - "method in the generated class definition" + f"Failed to generate class {key.value.invoke_name} needed for invoking " + "dbus-python methods" ) from err - return klass - - except DPClientGenerationError as err: # pragma: no cover - raise StratisCliGenerationError( - f"Failed to generate class {key.value.invoke_name} needed for invoking " - "dbus-python methods" - ) from err + assert False # pragma: no cover diff --git a/src/stratis_cli/_actions/_top.py b/src/stratis_cli/_actions/_top.py index d8195dc60..856996075 100644 --- a/src/stratis_cli/_actions/_top.py +++ b/src/stratis_cli/_actions/_top.py @@ -33,7 +33,7 @@ from .._stratisd_constants import ReportKey, StratisdErrors from ._connection import get_object from ._constants import TOP_OBJECT -from ._dynamic import ClassKey, make_dyn_class +from ._dynamic import ClassKey, Purpose, make_dyn_class from ._formatting import print_table @@ -45,7 +45,7 @@ def _fetch_keylist(proxy): :rtype: list of str :raises StratisCliEngineError: """ - Manager = make_dyn_class(ClassKey.MANAGER) + Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) (keys, return_code, message) = Manager.Methods.ListKeys(proxy, {}) if return_code != StratisdErrors.OK: # pragma: no cover @@ -68,7 +68,7 @@ def _add_update_key(proxy, key_desc, capture_key, *, keyfile_path): """ assert capture_key == (keyfile_path is None) - Manager = make_dyn_class(ClassKey.MANAGER) + Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) if capture_key: password = getpass(prompt="Enter key data followed by the return key: ") @@ -117,7 +117,7 @@ def get_report(namespace): """ if namespace.report_name == ReportKey.MANAGED_OBJECTS.value: - ObjectManager = make_dyn_class(ClassKey.OBJECT_MANAGER) + ObjectManager = make_dyn_class(ClassKey.OBJECT_MANAGER, Purpose.INVOKE) json_report = ObjectManager.Methods.GetManagedObjects( get_object(TOP_OBJECT), {} @@ -125,14 +125,14 @@ def get_report(namespace): else: if namespace.report_name == ReportKey.ENGINE_STATE.value: - Manager = make_dyn_class(ClassKey.MANAGER) + Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) (report, return_code, message) = Manager.Methods.EngineStateReport( get_object(TOP_OBJECT), {} ) else: - Report = make_dyn_class(ClassKey.REPORT) + Report = make_dyn_class(ClassKey.REPORT, Purpose.INVOKE) (report, return_code, message) = Report.Methods.GetReport( get_object(TOP_OBJECT), {"name": namespace.report_name} @@ -240,7 +240,7 @@ def unset_key(namespace): :raises StratisCliNoChangeError: :raises StratisCliIncoherenceError: """ - Manager = make_dyn_class(ClassKey.MANAGER) + Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) proxy = get_object(TOP_OBJECT) From 61d09fd78ee05382cdbba33df550467737ad8fb3 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 27 Nov 2023 21:00:02 -0500 Subject: [PATCH 07/14] Temporary no cover Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 96856ae24..d28b92a9b 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -118,7 +118,7 @@ def make_dyn_class(key, purpose): try: if key == ClassKey.MANAGER: _add_abs_path_assertion(klass, "CreatePool", "devices") - if key == ClassKey.POOL: + if key == ClassKey.POOL: # pragma: no cover _add_abs_path_assertion(klass, "InitCache", "devices") _add_abs_path_assertion(klass, "AddCacheDevs", "devices") _add_abs_path_assertion(klass, "AddDataDevs", "devices") From ca36488ec6dfb0e331893c19cbac3f05fc7332bc Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 29 Nov 2023 14:50:39 -0500 Subject: [PATCH 08/14] Simplify Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 57 +++++++++------------------- src/stratis_cli/_actions/_top.py | 14 +++---- 2 files changed, 24 insertions(+), 47 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index d28b92a9b..f2701f3c0 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -23,12 +23,7 @@ from dbus_python_client_gen import DPClientGenerationError, make_class from .._errors import StratisCliGenerationError -from ._constants import ( - FILESYSTEM_INTERFACE, - MANAGER_INTERFACE, - POOL_INTERFACE, - REPORT_INTERFACE, -) +from ._constants import MANAGER_INTERFACE, REPORT_INTERFACE from ._environment import get_timeout from ._introspect import SPECS @@ -39,33 +34,6 @@ ) -class ClassInfo: # pylint: disable=too-few-public-methods - """ - Information used to construct the dynamically generated class. - """ - - def __init__(self, interface_name, *, invoke_name=None): - """ - Initializer. - """ - self.interface_name = interface_name - self.invoke_name = invoke_name - - -class ClassKey(Enum): - """ - Keys for dynamically generated classes. - """ - - FILESYSTEM = ClassInfo(FILESYSTEM_INTERFACE, invoke_name="Filesystem") - MANAGER = ClassInfo(MANAGER_INTERFACE, invoke_name="Manager") - OBJECT_MANAGER = ClassInfo( - "org.freedesktop.DBus.ObjectManager", invoke_name="ObjectManager" - ) - POOL = ClassInfo(POOL_INTERFACE, invoke_name="Pool") - REPORT = ClassInfo(REPORT_INTERFACE, invoke_name="Report") - - class Purpose(Enum): """ Purpose of class to be created. @@ -76,6 +44,13 @@ class Purpose(Enum): SEARCH = 2 # search for object in GEtManagedObjects result +_LOOKUP = { + "Manager": (Purpose.INVOKE, MANAGER_INTERFACE), + "ObjectManager": (Purpose.INVOKE, "org.freedesktop.DBus.ObjectManager"), + "Report": (Purpose.INVOKE, REPORT_INTERFACE), +} + + def _add_abs_path_assertion(klass, method_name, key): """ Set method_name of method_klass to a new method which checks that the @@ -101,24 +76,26 @@ def new_method(proxy, args): setattr(method_class, method_name, new_method) -def make_dyn_class(key, purpose): +def make_dyn_class(name): """ Dynamically generate a class from introspection specification. - :param ClassKey key: key that identifies the class to make + :param str name: name of class to make """ + (purpose, interface_name) = _LOOKUP[name] + if purpose is Purpose.INVOKE: try: klass = make_class( - key.value.invoke_name, - ET.fromstring(SPECS[key.value.interface_name]), # nosec B314 + name, + ET.fromstring(SPECS[interface_name]), # nosec B314 TIMEOUT, ) try: - if key == ClassKey.MANAGER: + if name == "Manager": _add_abs_path_assertion(klass, "CreatePool", "devices") - if key == ClassKey.POOL: # pragma: no cover + if name == "Pool": # pragma: no cover _add_abs_path_assertion(klass, "InitCache", "devices") _add_abs_path_assertion(klass, "AddCacheDevs", "devices") _add_abs_path_assertion(klass, "AddDataDevs", "devices") @@ -135,7 +112,7 @@ def make_dyn_class(key, purpose): except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( - f"Failed to generate class {key.value.invoke_name} needed for invoking " + f"Failed to generate class {name} needed for invoking " "dbus-python methods" ) from err diff --git a/src/stratis_cli/_actions/_top.py b/src/stratis_cli/_actions/_top.py index 856996075..ca639b0de 100644 --- a/src/stratis_cli/_actions/_top.py +++ b/src/stratis_cli/_actions/_top.py @@ -33,7 +33,7 @@ from .._stratisd_constants import ReportKey, StratisdErrors from ._connection import get_object from ._constants import TOP_OBJECT -from ._dynamic import ClassKey, Purpose, make_dyn_class +from ._dynamic import make_dyn_class from ._formatting import print_table @@ -45,7 +45,7 @@ def _fetch_keylist(proxy): :rtype: list of str :raises StratisCliEngineError: """ - Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) + Manager = make_dyn_class("Manager") (keys, return_code, message) = Manager.Methods.ListKeys(proxy, {}) if return_code != StratisdErrors.OK: # pragma: no cover @@ -68,7 +68,7 @@ def _add_update_key(proxy, key_desc, capture_key, *, keyfile_path): """ assert capture_key == (keyfile_path is None) - Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) + Manager = make_dyn_class("Manager") if capture_key: password = getpass(prompt="Enter key data followed by the return key: ") @@ -117,7 +117,7 @@ def get_report(namespace): """ if namespace.report_name == ReportKey.MANAGED_OBJECTS.value: - ObjectManager = make_dyn_class(ClassKey.OBJECT_MANAGER, Purpose.INVOKE) + ObjectManager = make_dyn_class("ObjectManager") json_report = ObjectManager.Methods.GetManagedObjects( get_object(TOP_OBJECT), {} @@ -125,14 +125,14 @@ def get_report(namespace): else: if namespace.report_name == ReportKey.ENGINE_STATE.value: - Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) + Manager = make_dyn_class("Manager") (report, return_code, message) = Manager.Methods.EngineStateReport( get_object(TOP_OBJECT), {} ) else: - Report = make_dyn_class(ClassKey.REPORT, Purpose.INVOKE) + Report = make_dyn_class("Report") (report, return_code, message) = Report.Methods.GetReport( get_object(TOP_OBJECT), {"name": namespace.report_name} @@ -240,7 +240,7 @@ def unset_key(namespace): :raises StratisCliNoChangeError: :raises StratisCliIncoherenceError: """ - Manager = make_dyn_class(ClassKey.MANAGER, Purpose.INVOKE) + Manager = make_dyn_class("Manager") proxy = get_object(TOP_OBJECT) From a85e95c93e6c7603345c7ee37c6add6b3d72962a Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 29 Nov 2023 15:21:58 -0500 Subject: [PATCH 09/14] Memoize Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index f2701f3c0..4b5785c87 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -45,9 +45,9 @@ class Purpose(Enum): _LOOKUP = { - "Manager": (Purpose.INVOKE, MANAGER_INTERFACE), - "ObjectManager": (Purpose.INVOKE, "org.freedesktop.DBus.ObjectManager"), - "Report": (Purpose.INVOKE, REPORT_INTERFACE), + "Manager": (Purpose.INVOKE, MANAGER_INTERFACE, None), + "ObjectManager": (Purpose.INVOKE, "org.freedesktop.DBus.ObjectManager", None), + "Report": (Purpose.INVOKE, REPORT_INTERFACE, None), } @@ -82,7 +82,10 @@ def make_dyn_class(name): :param str name: name of class to make """ - (purpose, interface_name) = _LOOKUP[name] + (purpose, interface_name, klass) = _LOOKUP[name] + + if klass is not None: + return klass if purpose is Purpose.INVOKE: try: @@ -108,12 +111,12 @@ def make_dyn_class(name): "method in the generated class definition" ) from err - return klass - except DPClientGenerationError as err: # pragma: no cover raise StratisCliGenerationError( f"Failed to generate class {name} needed for invoking " "dbus-python methods" ) from err - assert False # pragma: no cover + _LOOKUP[name] = (purpose, interface_name, klass) + + return klass From 456d6e440dad1bd0fa0c8b2d461dee039a9604d6 Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 29 Nov 2023 15:57:41 -0500 Subject: [PATCH 10/14] Add no cover temporarily Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 4b5785c87..30c045900 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -87,7 +87,7 @@ def make_dyn_class(name): if klass is not None: return klass - if purpose is Purpose.INVOKE: + if purpose is Purpose.INVOKE: # pragma: no cover try: klass = make_class( name, From 20e4e2667961be60feb4a9cd654956fca9fcc535 Mon Sep 17 00:00:00 2001 From: mulhern Date: Sun, 3 Dec 2023 21:03:58 -0500 Subject: [PATCH 11/14] Use an anonymous function to get the interface specification Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 29 ++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 30c045900..40f4a5b2f 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -45,9 +45,23 @@ class Purpose(Enum): _LOOKUP = { - "Manager": (Purpose.INVOKE, MANAGER_INTERFACE, None), - "ObjectManager": (Purpose.INVOKE, "org.freedesktop.DBus.ObjectManager", None), - "Report": (Purpose.INVOKE, REPORT_INTERFACE, None), + "Manager": ( + Purpose.INVOKE, + lambda: ET.fromstring(SPECS[MANAGER_INTERFACE]), # nosec B314 + None, + ), + "ObjectManager": ( + Purpose.INVOKE, + lambda: ET.fromstring( + SPECS["org.freedesktop.DBus.ObjectManager"] + ), # nosec B314 + None, + ), + "Report": ( + Purpose.INVOKE, + lambda: ET.fromstring(SPECS[REPORT_INTERFACE]), # nosec B314 + None, + ), } @@ -82,16 +96,18 @@ def make_dyn_class(name): :param str name: name of class to make """ - (purpose, interface_name, klass) = _LOOKUP[name] + (purpose, interface_func, klass) = _LOOKUP[name] if klass is not None: return klass + assert interface_func is not None + if purpose is Purpose.INVOKE: # pragma: no cover try: klass = make_class( name, - ET.fromstring(SPECS[interface_name]), # nosec B314 + interface_func(), TIMEOUT, ) @@ -117,6 +133,7 @@ def make_dyn_class(name): "dbus-python methods" ) from err - _LOOKUP[name] = (purpose, interface_name, klass) + # set the function to None since the class has been obtained + _LOOKUP[name] = (purpose, None, klass) return klass From 48d98cbf9abbcf976292a6407628964e7422e461 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 4 Dec 2023 12:36:23 -0500 Subject: [PATCH 12/14] Add support for Manager0 Signed-off-by: mulhern --- src/stratis_cli/_actions/_dynamic.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/stratis_cli/_actions/_dynamic.py b/src/stratis_cli/_actions/_dynamic.py index 40f4a5b2f..9ab01e496 100644 --- a/src/stratis_cli/_actions/_dynamic.py +++ b/src/stratis_cli/_actions/_dynamic.py @@ -33,6 +33,14 @@ os.environ.get("STRATIS_DBUS_TIMEOUT", DBUS_TIMEOUT_SECONDS * 1000) ) +MANAGER_SPEC = """ + + + + + +""" + class Purpose(Enum): """ @@ -50,6 +58,11 @@ class Purpose(Enum): lambda: ET.fromstring(SPECS[MANAGER_INTERFACE]), # nosec B314 None, ), + "Manager0": ( + Purpose.INVOKE, + lambda: ET.fromstring(MANAGER_SPEC), # nosec B314 + None, + ), "ObjectManager": ( Purpose.INVOKE, lambda: ET.fromstring( From 89eeb31a50edff1279b2e2a42ac8167661a2d054 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 4 Dec 2023 12:39:44 -0500 Subject: [PATCH 13/14] Use Manager0 Signed-off-by: mulhern --- pyproject.toml | 2 +- src/stratis_cli/_actions/_stratisd_version.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6b1808967..c862c2e3f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,4 +3,4 @@ requires = ["setuptools"] build-backend = "setuptools.build_meta" [tool.pylint] -good-names="Manager,ObjectManager,Report" +good-names="Manager0,Manager,ObjectManager,Report" diff --git a/src/stratis_cli/_actions/_stratisd_version.py b/src/stratis_cli/_actions/_stratisd_version.py index e4de864c4..9409a9671 100644 --- a/src/stratis_cli/_actions/_stratisd_version.py +++ b/src/stratis_cli/_actions/_stratisd_version.py @@ -21,6 +21,7 @@ from .._errors import StratisCliStratisdVersionError from ._connection import get_object from ._constants import MAXIMUM_STRATISD_VERSION, MINIMUM_STRATISD_VERSION, TOP_OBJECT +from ._dynamic import make_dyn_class def check_stratisd_version(): @@ -30,8 +31,7 @@ def check_stratisd_version(): :raises StratisCliStratisdVersionError """ - # pylint: disable=import-outside-toplevel - from ._data import Manager0 + Manager0 = make_dyn_class("Manager0") version_spec = SpecifierSet(f">={MINIMUM_STRATISD_VERSION}") & SpecifierSet( f"<{MAXIMUM_STRATISD_VERSION}" From 044e0833004273296866921089e909dbe22e3ddb Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 4 Dec 2023 13:26:12 -0500 Subject: [PATCH 14/14] Sort out some tests Signed-off-by: mulhern --- tests/whitebox/integration/test_stratis.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/whitebox/integration/test_stratis.py b/tests/whitebox/integration/test_stratis.py index 0d128374d..1c5112f90 100644 --- a/tests/whitebox/integration/test_stratis.py +++ b/tests/whitebox/integration/test_stratis.py @@ -23,6 +23,7 @@ # isort: LOCAL from stratis_cli import StratisCliErrorCodes, run +from stratis_cli._actions import _dynamic from stratis_cli._errors import StratisCliStratisdVersionError from ._misc import RUNNER, TEST_RUNNER, RunTestCase, SimTestCase @@ -76,15 +77,14 @@ def test_outdated_stratisd_version(self): Verify that an outdated version of stratisd will produce a StratisCliStratisdVersionError. """ - # pylint: disable=import-outside-toplevel - # isort: LOCAL - from stratis_cli._actions import _data + _dynamic.make_dyn_class("Manager0") command_line = ["--propagate", "daemon", "version"] - # pylint: disable=protected-access with patch.object( - _data.Manager0.Properties.Version, + _dynamic._LOOKUP["Manager0"][ # pylint: disable=protected-access + 2 + ].Properties.Version, "Get", return_value="1.0.0", ): @@ -107,12 +107,14 @@ def test_catch_keyboard_exception(self): at the calling method generated by dbus-python-client-gen. """ - # pylint: disable=import-outside-toplevel - # isort: LOCAL - from stratis_cli._actions import _data + _dynamic.make_dyn_class("Manager0") with patch.object( - _data.Manager0.Properties.Version, "Get", side_effect=KeyboardInterrupt() + _dynamic._LOOKUP["Manager0"][ # pylint: disable=protected-access + 2 + ].Properties.Version, + "Get", + side_effect=KeyboardInterrupt(), ): with self.assertRaises(KeyboardInterrupt): run()(["daemon", "version"])