From c4dd81671f631e0b5d8d6102a4050fa23645be3f Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 7 Apr 2025 16:02:24 +0000 Subject: [PATCH 1/6] Auto Local System Clients --- CHANGELOG.rst | 7 +++++++ brewtils/plugin.py | 21 +++++++++++++++++++++ brewtils/rest/system_client.py | 25 +++++++++++++++++++++---- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d152f730..ac80f534 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,13 @@ Brewtils Changelog ================== +3.31.0 +------ +TBD + +- Added support to auto generate requests when calling functions locally that are annotated with command decorators. This only supports + plugin classes that extend `object` and does not support recursive command loops. + 3.29.1 ------ 12/31/24 diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 0d0e0213..baa716ec 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -43,6 +43,7 @@ ) from brewtils.resolvers.manager import ResolutionManager from brewtils.rest.easy_client import EasyClient +from brewtils.rest.system_client import SystemClient from brewtils.specification import _CONNECTION_SPEC # This is what enables request nesting to work easily @@ -437,6 +438,26 @@ def _set_client(self, new_client): client_clazz._prefix_topic = self._system.prefix_topic client_clazz._requires = self._system.requires client_clazz._current_request = client_clazz.current_request + + # Can only inject System Clients if the Client inherits from a class object + # TODO: Should we add a configuration to explicitly disable this? + if issubclass(client_clazz, object): + + def system_client_getattribute(self, name): + try: + if not name.startswith("_"): + request = get_current_request_read_only() + # Does not support recursive calls + if request and request.command != name: + if self.__class__._bg_commands: + for bg_command in self.__class__._bg_commands: + if bg_command.name == name: + return getattr(SystemClient(), name) + except Exception: + pass + return object.__getattribute__(self, name) + + new_client.__class__.__getattribute__ = system_client_getattribute except TypeError: if sys.version_info.major != 2: raise diff --git a/brewtils/rest/system_client.py b/brewtils/rest/system_client.py index 374f673f..f362fe60 100644 --- a/brewtils/rest/system_client.py +++ b/brewtils/rest/system_client.py @@ -407,10 +407,27 @@ def send_bg_request(self, *args, **kwargs): # tried to pass a parameter without a key: # client.command_name(param) if args: - raise RequestProcessException( - "Using positional arguments when creating a request is not allowed. " - "Please use keyword arguments instead." - ) + if self.target_self: + _command = self._commands[kwargs["_command"]] + if _command: + arg_counter = 0 + for arg in args: + if arg_counter < len(_command.parameters): + if _command.parameters[arg_counter].key in kwargs: + raise RequestProcessException( + "Keyword argument overlapped with provided positional argument. Please use all keyword arguments instead." + ) + kwargs[_command.parameters[arg_counter].key] = arg + arg_counter = arg_counter + 1 + else: + raise RequestProcessException( + "More positional arguments provided that command parameters. Please use all keyword arguments instead." + ) + else: + raise RequestProcessException( + "Using positional arguments when creating a request is not allowed. " + "Please use keyword arguments instead." + ) # Need to pop here, otherwise we'll try to send as a request parameter raise_on_error = kwargs.pop("_raise_on_error", self._raise_on_error) From 7032421e81703ef2955ba6d377b3244ea64d0a3f Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 14 Apr 2025 13:24:10 +0000 Subject: [PATCH 2/6] Added feature to disable auto self client --- brewtils/plugin.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index baa716ec..62cfc093 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -95,6 +95,7 @@ class Plugin(object): - ``require`` - ``requires`` - ``requires_timeout`` + - ``auto_self_client`` (defaults to "true") Connection information tells the Plugin how to communicate with Beer-garden. The most important of these is the ``bg_host`` (to tell the plugin where to find the @@ -207,6 +208,9 @@ class Plugin(object): prefix_topic (str): Prefix for Generated Command Topics + auto_self_client (bool): Whether to automatically invoke SystemClient for local + system commands via self + logger (:py:class:`logging.Logger`): Logger that will be used by the Plugin. Passing a logger will prevent the Plugin from preforming any additional logging configuration. @@ -240,6 +244,8 @@ def __init__(self, client=None, system=None, logger=None, **kwargs): self._custom_logger = False self._logger = self._setup_logging(logger=logger, **kwargs) + self._auto_self_client = kwargs.pop("auto_self_client", True) + # Need to pop out shutdown functions because these are not processed # until shutdown self.shutdown_functions = [] @@ -441,7 +447,7 @@ def _set_client(self, new_client): # Can only inject System Clients if the Client inherits from a class object # TODO: Should we add a configuration to explicitly disable this? - if issubclass(client_clazz, object): + if self._auto_self_client and issubclass(client_clazz, object): def system_client_getattribute(self, name): try: From 87388a710971cc70ca0aa9f3a31c54dc5b736855 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 14 Apr 2025 13:26:56 +0000 Subject: [PATCH 3/6] formatting --- brewtils/rest/system_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/brewtils/rest/system_client.py b/brewtils/rest/system_client.py index f362fe60..e07bd224 100644 --- a/brewtils/rest/system_client.py +++ b/brewtils/rest/system_client.py @@ -415,13 +415,19 @@ def send_bg_request(self, *args, **kwargs): if arg_counter < len(_command.parameters): if _command.parameters[arg_counter].key in kwargs: raise RequestProcessException( - "Keyword argument overlapped with provided positional argument. Please use all keyword arguments instead." + ( + "Keyword argument overlapped with provided positional " + "argument. Please use all keyword arguments instead." + ) ) kwargs[_command.parameters[arg_counter].key] = arg arg_counter = arg_counter + 1 else: raise RequestProcessException( - "More positional arguments provided that command parameters. Please use all keyword arguments instead." + ( + "More positional arguments provided that command parameters. " + "Please use all keyword arguments instead." + ) ) else: raise RequestProcessException( From c2515d3e81365b3fb6c0e76b74102cd25ab36d20 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:20:25 +0000 Subject: [PATCH 4/6] Adding config parsing and testing --- brewtils/specification.py | 8 ++++++++ test/plugin_test.py | 2 ++ 2 files changed, 10 insertions(+) diff --git a/brewtils/specification.py b/brewtils/specification.py index 2cbf112b..37aab8d2 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -215,6 +215,14 @@ def _is_json_dict(s): } _PLUGIN_SPEC = { + "auto_self_client": { + "type": "bool", + "description": ( + "Whether to automatically invoke SystemClient for local" + "system commands via self" + ), + "default": True, + }, "instance_name": { "type": "str", "description": "The instance name", diff --git a/test/plugin_test.py b/test/plugin_test.py index eae93048..1b86c1c3 100644 --- a/test/plugin_test.py +++ b/test/plugin_test.py @@ -201,6 +201,7 @@ def test_env(self, client, bg_system): os.environ["BG_GROUP"] = "GroupA" os.environ["BG_PREFIX_TOPIC"] = "custom.topic" os.environ["BG_REQUIRE"] = "SystemA" + os.environ["BG_AUTO_SELF_CLIENT"] = "False" plugin = Plugin(client, system=bg_system, max_concurrent=1) @@ -210,6 +211,7 @@ def test_env(self, client, bg_system): assert plugin._config.ssl_enabled is False assert plugin._config.ca_verify is False assert plugin._config.prefix_topic == "custom.topic" + assert plugin._config.auto_self_client is False assert "GroupA" == plugin._config.group assert "SystemA" in plugin._config.require From 9654ee65d59c1465edb1074769981c655bafa4bf Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 20 Apr 2026 15:40:03 +0000 Subject: [PATCH 5/6] Set default to False --- CHANGELOG.rst | 3 ++- brewtils/plugin.py | 6 ++++-- brewtils/specification.py | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 35f2cd2f..82faa79e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,8 @@ Brewtils Changelog TBD - Added support to auto generate requests when calling functions locally that are annotated with command decorators. This only supports - plugin classes that extend `object` and does not support recursive command loops. (#547) + plugin classes that extend `object` and does not support recursive command loops. Defaults to disabled. (#547) + Examples: Plugin(..., auto_self_client=True) - Added support for SystemClient to define choice_validation_enabled flag during initialization or command execution. Default behavior is to skip choice validation if a parent request is present. (#585) Examples: SystemClient(..., choice_validation_enabled=True) or SystemClient().call_command(..., _choice_validation_enabled=True) diff --git a/brewtils/plugin.py b/brewtils/plugin.py index 751b46b7..bb9d606a 100644 --- a/brewtils/plugin.py +++ b/brewtils/plugin.py @@ -139,7 +139,7 @@ class Plugin(object): - ``require`` - ``requires`` - ``requires_timeout`` - - ``auto_self_client`` (defaults to "true") + - ``auto_self_client`` (defaults to "false") Connection information tells the Plugin how to communicate with Beer-garden. The most important of these is the ``bg_host`` (to tell the plugin where to find the @@ -288,7 +288,9 @@ def __init__(self, client=None, system=None, logger=None, **kwargs): self._custom_logger = False self._logger = self._setup_logging(logger=logger, **kwargs) - self._auto_self_client = kwargs.pop("auto_self_client", True) + self._auto_self_client = kwargs.pop("auto_self_client", False) + if not isinstance(self._auto_self_client, bool): + self._auto_self_client = str(self._auto_self_client).lower() == "true" # Need to pop out shutdown functions because these are not processed # until shutdown diff --git a/brewtils/specification.py b/brewtils/specification.py index 37aab8d2..6c5dd289 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -221,7 +221,7 @@ def _is_json_dict(s): "Whether to automatically invoke SystemClient for local" "system commands via self" ), - "default": True, + "default": False, }, "instance_name": { "type": "str", From 8871714950dcfd6d38c8447edb2f4b4f2225d334 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:19:44 -0400 Subject: [PATCH 6/6] Update CHANGELOG for command decorators feature Clarified the description of auto-generated requests feature and its default state. --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 82faa79e..07915912 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,7 @@ Brewtils Changelog TBD - Added support to auto generate requests when calling functions locally that are annotated with command decorators. This only supports - plugin classes that extend `object` and does not support recursive command loops. Defaults to disabled. (#547) + plugin classes that extend `object` and does not support recursive command loops. This is disabled by default (#547) Examples: Plugin(..., auto_self_client=True) - Added support for SystemClient to define choice_validation_enabled flag during initialization or command execution. Default behavior is to skip choice validation if a parent request is present. (#585)