From a7fa37bbb88c7b13bca181f01f792aaec35f6e89 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:55:50 +0000 Subject: [PATCH 1/3] Fixed missing requester for spawned threads of SystemClient --- CHANGELOG.rst | 2 ++ brewtils/rest/system_client.py | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a1d4b1bd..7e78428a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,8 @@ Brewtils Changelog TBD - Expanded PublishClient to support Registering and Unregistering commands to Topics after a plugin has been initialized +- Fixed bug where threading a SystemClient within a plugin lost current_request context and failed to map `requester`, + SystemClient will still drop the current_request context,but the requester field can be provided via `_requester` 3.27.1 ------ diff --git a/brewtils/rest/system_client.py b/brewtils/rest/system_client.py index bfcd216b..cb2a1b3f 100644 --- a/brewtils/rest/system_client.py +++ b/brewtils/rest/system_client.py @@ -595,13 +595,16 @@ def _construct_bg_request(self, **kwargs): publish = kwargs.pop("_publish", None) topic = kwargs.pop("_topic", None) propagate = kwargs.pop("_propagate", None) + requester = kwargs.pop("_requester", None) - if parent: + if ( + not requester + and parent + and getattr(brewtils.plugin.request_context, "current_request", None) + ): requester = getattr( brewtils.plugin.request_context.current_request, "requester", None ) - else: - requester = None if system_display: metadata["system_display_name"] = system_display From 2ae729f8673c17a33b86f3e4d846742840c116b3 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:05:10 +0000 Subject: [PATCH 2/3] Requester Testing --- test/rest/system_client_test.py | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/rest/system_client_test.py b/test/rest/system_client_test.py index 5bdacdbc..fb3925be 100644 --- a/test/rest/system_client_test.py +++ b/test/rest/system_client_test.py @@ -304,6 +304,51 @@ def test_missing_field(self, monkeypatch, client, remove_kwarg): with pytest.raises(ValidationError): client._construct_bg_request(**kwargs) + def test_requester_field(self, monkeypatch, client, remove_kwarg): + monkeypatch.setattr(client, "_resolve_parameters", Mock()) + + kwargs = { + "_command": "", + "_system_name": "", + "_system_version": "", + "_instance_name": "", + "_requester":"test" + } + + request = client._construct_bg_request(**kwargs) + assert request.requester == "test" + + def test_requester_from_parent_field(self, monkeypatch, client, remove_kwarg): + monkeypatch.setattr(client, "_resolve_parameters", Mock()) + monkeypatch.setattr( + brewtils.plugin, "request_context", Mock(current_request=Mock(id="1", requester="test")) + ) + + kwargs = { + "_command": "", + "_system_name": "", + "_system_version": "", + "_instance_name": "", + } + + request = client._construct_bg_request(**kwargs) + assert request.requester == "test" + + def test_no_requester_from_provided_parent(self, monkeypatch, client, remove_kwarg): + monkeypatch.setattr(client, "_resolve_parameters", Mock()) + + kwargs = { + "_command": "", + "_system_name": "", + "_system_version": "", + "_instance_name": "", + "_parent":Mock(id="1"), + } + + request = client._construct_bg_request(**kwargs) + assert request.requester is None + + def test_positional_parameter(self, client, easy_client, mock_success): easy_client.create_request.return_value = mock_success From 166f9623ba3f2bf5068297e57293d1eff359e51e Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:06:54 +0000 Subject: [PATCH 3/3] formatting --- test/rest/system_client_test.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/rest/system_client_test.py b/test/rest/system_client_test.py index fb3925be..b1411caf 100644 --- a/test/rest/system_client_test.py +++ b/test/rest/system_client_test.py @@ -304,7 +304,7 @@ def test_missing_field(self, monkeypatch, client, remove_kwarg): with pytest.raises(ValidationError): client._construct_bg_request(**kwargs) - def test_requester_field(self, monkeypatch, client, remove_kwarg): + def test_requester_field(self, monkeypatch, client): monkeypatch.setattr(client, "_resolve_parameters", Mock()) kwargs = { @@ -312,18 +312,20 @@ def test_requester_field(self, monkeypatch, client, remove_kwarg): "_system_name": "", "_system_version": "", "_instance_name": "", - "_requester":"test" + "_requester": "test", } request = client._construct_bg_request(**kwargs) assert request.requester == "test" - def test_requester_from_parent_field(self, monkeypatch, client, remove_kwarg): + def test_requester_from_parent_field(self, monkeypatch, client): monkeypatch.setattr(client, "_resolve_parameters", Mock()) monkeypatch.setattr( - brewtils.plugin, "request_context", Mock(current_request=Mock(id="1", requester="test")) + brewtils.plugin, + "request_context", + Mock(current_request=Mock(id="1", requester="test")), ) - + kwargs = { "_command": "", "_system_name": "", @@ -334,7 +336,7 @@ def test_requester_from_parent_field(self, monkeypatch, client, remove_kwarg): request = client._construct_bg_request(**kwargs) assert request.requester == "test" - def test_no_requester_from_provided_parent(self, monkeypatch, client, remove_kwarg): + def test_no_requester_from_provided_parent(self, monkeypatch, client): monkeypatch.setattr(client, "_resolve_parameters", Mock()) kwargs = { @@ -342,12 +344,11 @@ def test_no_requester_from_provided_parent(self, monkeypatch, client, remove_kwa "_system_name": "", "_system_version": "", "_instance_name": "", - "_parent":Mock(id="1"), + "_parent": Mock(id="1"), } request = client._construct_bg_request(**kwargs) assert request.requester is None - def test_positional_parameter(self, client, easy_client, mock_success): easy_client.create_request.return_value = mock_success