From f5b5ee29bf9b6ee56125a94816d71b5aad7eb6c0 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Mon, 19 May 2025 16:55:08 +0530 Subject: [PATCH 1/8] feat: Add convenience methods for adding single auth token getter to tools --- .../src/toolbox_core/sync_tool.py | 22 +++++++++++++++++++ .../toolbox-core/src/toolbox_core/tool.py | 22 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/packages/toolbox-core/src/toolbox_core/sync_tool.py b/packages/toolbox-core/src/toolbox_core/sync_tool.py index 1b21d722..a47767ca 100644 --- a/packages/toolbox-core/src/toolbox_core/sync_tool.py +++ b/packages/toolbox-core/src/toolbox_core/sync_tool.py @@ -154,6 +154,28 @@ def add_auth_token_getters( new_async_tool = self.__async_tool.add_auth_token_getters(auth_token_getters) return ToolboxSyncTool(new_async_tool, self.__loop, self.__thread) + def add_auth_token_getter( + self, auth_source: str, get_id_token: Callable[[], str] + ) -> "ToolboxSyncTool": + """ + Registers auth token getter functions that are used for AuthServices + when tools are invoked. + + Args: + auth_source: The name of the authentication source. + get_id_token: A function that returns the ID token. + + Returns: + A new ToolboxSyncTool instance with the specified authentication + token getters registered. + + Raises: + ValueError: If the auth source has already been registered either to + the tool or to the corresponding client. + + """ + return self.add_auth_token_getters({auth_source: get_id_token}) + def bind_params( self, bound_params: Mapping[str, Union[Callable[[], Any], Any]] ) -> "ToolboxSyncTool": diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 5a9bca6e..44d53e2a 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -346,6 +346,28 @@ def add_auth_token_getters( required_authz_tokens=tuple(new_req_authz_tokens), ) + def add_auth_token_getter( + self, auth_source: str, get_id_token: Callable[[], str] + ) -> "ToolboxTool": + """ + Registers auth token getter functions that are used for AuthServices + when tools are invoked. + + Args: + auth_source: The name of the authentication source. + get_id_token: A function that returns the ID token. + + Returns: + A new ToolboxTool instance with the specified authentication token + getters registered. + + Raises + ValueError: If the auth source has already been registered either + to the tool or to the corresponding client. + + """ + return self.add_auth_token_getters({auth_source: get_id_token}) + def bind_params( self, bound_params: Mapping[str, Union[Callable[[], Any], Any]] ) -> "ToolboxTool": From 37720388ec25c4a5660333132713f895d5301ab1 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Mon, 19 May 2025 17:24:26 +0530 Subject: [PATCH 2/8] chore: Add unit test coverage --- packages/toolbox-core/tests/test_sync_tool.py | 33 +++++++++++++++++++ packages/toolbox-core/tests/test_tool.py | 29 ++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/packages/toolbox-core/tests/test_sync_tool.py b/packages/toolbox-core/tests/test_sync_tool.py index 84d00ea4..dd481bab 100644 --- a/packages/toolbox-core/tests/test_sync_tool.py +++ b/packages/toolbox-core/tests/test_sync_tool.py @@ -237,6 +237,39 @@ def test_toolbox_sync_tool_add_auth_token_getters( ) +def test_toolbox_sync_tool_add_auth_token_getter( + toolbox_sync_tool: ToolboxSyncTool, + mock_async_tool: MagicMock, + event_loop: asyncio.AbstractEventLoop, + mock_thread: MagicMock, +): + """Tests the add_auth_token_getter method.""" + auth_service = "service1" + auth_token_getter = lambda: "token1" + + new_mock_async_tool = mock_async_tool.add_auth_token_getters.return_value + new_mock_async_tool.__name__ = "new_async_tool_with_auth" + + new_sync_tool = toolbox_sync_tool.add_auth_token_getter( + auth_service, auth_token_getter + ) + + mock_async_tool.add_auth_token_getters.assert_called_once_with( + {auth_service: auth_token_getter} + ) + + assert isinstance(new_sync_tool, ToolboxSyncTool) + assert new_sync_tool is not toolbox_sync_tool + assert new_sync_tool._ToolboxSyncTool__async_tool is new_mock_async_tool + assert new_sync_tool._ToolboxSyncTool__loop is event_loop # Should be the same loop + assert ( + new_sync_tool._ToolboxSyncTool__thread is mock_thread + ) # Should be the same thread + assert ( + new_sync_tool.__qualname__ == f"ToolboxSyncTool.{new_mock_async_tool.__name__}" + ) + + def test_toolbox_sync_tool_bind_params( toolbox_sync_tool: ToolboxSyncTool, mock_async_tool: MagicMock, diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index c64149f5..4552fd1c 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -502,6 +502,35 @@ def test_add_auth_token_getters_unused_token( tool_instance.add_auth_token_getters(unused_auth_getters) +def test_add_auth_token_getter_unused_token( + http_session: ClientSession, + sample_tool_params: list[ParameterSchema], + sample_tool_description: str, + unused_auth_getters: Mapping[str, Callable[[], str]], +): + """ + Tests ValueError when add_auth_token_getters is called with a getter for + an unused authentication service. + """ + tool_instance = ToolboxTool( + session=http_session, + base_url=HTTPS_BASE_URL, + name=TEST_TOOL_NAME, + description=sample_tool_description, + params=sample_tool_params, + required_authn_params={}, + required_authz_tokens=[], + auth_service_token_getters={}, + bound_params={}, + client_headers={}, + ) + + expected_error_message = "Authentication source\(s\) \`unused-auth-service\` unused by tool \`sample_tool\`." + + with pytest.raises(ValueError, match=expected_error_message): + tool_instance.add_auth_token_getter(next(iter(unused_auth_getters)), unused_auth_getters[next(iter(unused_auth_getters))]) + + def test_toolbox_tool_underscore_name_property(toolbox_tool: ToolboxTool): """Tests the _name property.""" assert toolbox_tool._name == TEST_TOOL_NAME From 337631eb6948c389f48fb73780ba7f9c8e843b64 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Mon, 19 May 2025 17:24:41 +0530 Subject: [PATCH 3/8] chore: Delint --- packages/toolbox-core/tests/test_tool.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/toolbox-core/tests/test_tool.py b/packages/toolbox-core/tests/test_tool.py index 4552fd1c..0c624657 100644 --- a/packages/toolbox-core/tests/test_tool.py +++ b/packages/toolbox-core/tests/test_tool.py @@ -528,7 +528,10 @@ def test_add_auth_token_getter_unused_token( expected_error_message = "Authentication source\(s\) \`unused-auth-service\` unused by tool \`sample_tool\`." with pytest.raises(ValueError, match=expected_error_message): - tool_instance.add_auth_token_getter(next(iter(unused_auth_getters)), unused_auth_getters[next(iter(unused_auth_getters))]) + tool_instance.add_auth_token_getter( + next(iter(unused_auth_getters)), + unused_auth_getters[next(iter(unused_auth_getters))], + ) def test_toolbox_tool_underscore_name_property(toolbox_tool: ToolboxTool): From 3d7f21dafb56c7dabcc958cc169b870823746786 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Mon, 19 May 2025 17:29:25 +0530 Subject: [PATCH 4/8] docs: Add to README --- packages/toolbox-core/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolbox-core/README.md b/packages/toolbox-core/README.md index 74030312..4444ad6e 100644 --- a/packages/toolbox-core/README.md +++ b/packages/toolbox-core/README.md @@ -381,7 +381,7 @@ loaded. This modifies the specific tool instance. toolbox = ToolboxClient("http://127.0.0.1:5000") tool = await toolbox.load_tool("my-tool") -auth_tool = tool.add_auth_token_getters({"my_auth": get_auth_token}) # Single token +auth_tool = tool.add_auth_token_getter("my_auth", get_auth_token) # Single token # OR From 6e52dcb0ce8dea44d88b8cb8c2f7b2277706bdfa Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 20 May 2025 12:31:24 +0530 Subject: [PATCH 5/8] docs: Fix and improve docstrings --- .../src/toolbox_core/sync_tool.py | 28 +++++++++++---- .../toolbox-core/src/toolbox_core/tool.py | 34 ++++++++++++------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/sync_tool.py b/packages/toolbox-core/src/toolbox_core/sync_tool.py index a47767ca..71fde21f 100644 --- a/packages/toolbox-core/src/toolbox_core/sync_tool.py +++ b/packages/toolbox-core/src/toolbox_core/sync_tool.py @@ -139,16 +139,20 @@ def add_auth_token_getters( auth_token_getters: Mapping[str, Callable[[], str]], ) -> "ToolboxSyncTool": """ - Registers an auth token getter function that is used for AuthServices when tools - are invoked. + Registers auth token getter functions that are used for AuthServices + when tools are invoked. Args: auth_token_getters: A mapping of authentication service names to callables that return the corresponding authentication token. Returns: - A new ToolboxSyncTool instance with the specified authentication token - getters registered. + A new ToolboxSyncTool instance with the specified authentication + token getters registered. + + Raises: + ValueError: If an auth source has already been registered either to + the tool or to the corresponding client. """ new_async_tool = self.__async_tool.add_auth_token_getters(auth_token_getters) @@ -158,7 +162,7 @@ def add_auth_token_getter( self, auth_source: str, get_id_token: Callable[[], str] ) -> "ToolboxSyncTool": """ - Registers auth token getter functions that are used for AuthServices + Registers an auth token getter function that is used for AuthService when tools are invoked. Args: @@ -167,7 +171,7 @@ def add_auth_token_getter( Returns: A new ToolboxSyncTool instance with the specified authentication - token getters registered. + token getter registered. Raises: ValueError: If the auth source has already been registered either to @@ -188,6 +192,11 @@ def bind_params( Returns: A new ToolboxSyncTool instance with the specified parameters bound. + + Raises: + ValueError: If a parameter is already bound or is not defined by the + tool's definition. + """ new_async_tool = self.__async_tool.bind_params(bound_params) @@ -199,7 +208,7 @@ def bind_param( param_value: Union[Callable[[], Any], Any], ) -> "ToolboxSyncTool": """ - Binds a parameter to the value or callables that produce it. + Binds a parameter to the value or callable that produce the value. Args: param_name: The name of the bound parameter. @@ -208,5 +217,10 @@ def bind_param( Returns: A new ToolboxSyncTool instance with the specified parameter bound. + + Raises: + ValueError: If the parameter is already bound or is not defined by + the tool's definition. + """ return self.bind_params({param_name: param_value}) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 44d53e2a..a0d3226c 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -281,8 +281,8 @@ def add_auth_token_getters( auth_token_getters: Mapping[str, Callable[[], str]], ) -> "ToolboxTool": """ - Registers an auth token getter function that is used for AuthServices when tools - are invoked. + Registers auth token getter functions that are used for AuthServices + when tools are invoked. Args: auth_token_getters: A mapping of authentication service names to @@ -292,9 +292,9 @@ def add_auth_token_getters( A new ToolboxTool instance with the specified authentication token getters registered. - Raises - ValueError: If the auth source has already been registered either - to the tool or to the corresponding client. + Raises: + ValueError: If an auth source has already been registered either to + the tool or to the corresponding client. """ # throw an error if the authentication source is already registered @@ -350,7 +350,7 @@ def add_auth_token_getter( self, auth_source: str, get_id_token: Callable[[], str] ) -> "ToolboxTool": """ - Registers auth token getter functions that are used for AuthServices + Registers an auth token getter function that is used for AuthServices when tools are invoked. Args: @@ -359,11 +359,11 @@ def add_auth_token_getter( Returns: A new ToolboxTool instance with the specified authentication token - getters registered. + getter registered. - Raises - ValueError: If the auth source has already been registered either - to the tool or to the corresponding client. + Raises: + ValueError: If the auth source has already been registered either to + the tool or to the corresponding client. """ return self.add_auth_token_getters({auth_source: get_id_token}) @@ -380,6 +380,11 @@ def bind_params( Returns: A new ToolboxTool instance with the specified parameters bound. + + Raises: + ValueError: If a parameter is already bound or is not defined by the + tool's definition. + """ param_names = set(p.name for p in self.__params) for name in bound_params.keys(): @@ -411,7 +416,7 @@ def bind_param( param_value: Union[Callable[[], Any], Any], ) -> "ToolboxTool": """ - Binds a parameter to the value or callables that produce it. + Binds a parameter to the value or callable that produce the value. Args: param_name: The name of the bound parameter. @@ -419,6 +424,11 @@ def bind_param( returns the value. Returns: - A new ToolboxTool instance with the specified parameters bound. + A new ToolboxTool instance with the specified parameter bound. + + Raises: + ValueError: If the parameter is already bound or is not defined by + the tool's definition. + """ return self.bind_params({param_name: param_value}) From b60c2900227a0ec176c8adb101f4979b8b930f21 Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 20 May 2025 12:34:50 +0530 Subject: [PATCH 6/8] docs: Improve docstring --- packages/toolbox-core/src/toolbox_core/tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index a0d3226c..80d64473 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -350,7 +350,7 @@ def add_auth_token_getter( self, auth_source: str, get_id_token: Callable[[], str] ) -> "ToolboxTool": """ - Registers an auth token getter function that is used for AuthServices + Registers an auth token getter function that is used for AuthService when tools are invoked. Args: From 8b61edeaea846dbfa7ab0bf72a75d46eec1ffc3b Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 20 May 2025 12:41:41 +0530 Subject: [PATCH 7/8] chore: Delint --- packages/toolbox-core/src/toolbox_core/tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolbox-core/src/toolbox_core/tool.py b/packages/toolbox-core/src/toolbox_core/tool.py index 80d64473..64dd689a 100644 --- a/packages/toolbox-core/src/toolbox_core/tool.py +++ b/packages/toolbox-core/src/toolbox_core/tool.py @@ -380,7 +380,7 @@ def bind_params( Returns: A new ToolboxTool instance with the specified parameters bound. - + Raises: ValueError: If a parameter is already bound or is not defined by the tool's definition. From 127d0bee6fd9493a372873d0cdb4d902e58c945c Mon Sep 17 00:00:00 2001 From: Anubhav Dhawan Date: Tue, 20 May 2025 12:48:47 +0530 Subject: [PATCH 8/8] docs: Improve function docstrings and README (#246) * docs: Improve function docstrings in tool classes * docs: Add singular bind param method to README --- packages/toolbox-core/README.md | 4 ++++ packages/toolbox-core/src/toolbox_core/sync_tool.py | 11 +++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/toolbox-core/README.md b/packages/toolbox-core/README.md index 4444ad6e..0147c45c 100644 --- a/packages/toolbox-core/README.md +++ b/packages/toolbox-core/README.md @@ -459,6 +459,10 @@ specific tool instance. toolbox = ToolboxClient("http://127.0.0.1:5000") tool = await toolbox.load_tool("my-tool") +bound_tool = tool.bind_param("param", "value") + +# OR + bound_tool = tool.bind_params({"param": "value"}) ``` diff --git a/packages/toolbox-core/src/toolbox_core/sync_tool.py b/packages/toolbox-core/src/toolbox_core/sync_tool.py index 71fde21f..5a545d9f 100644 --- a/packages/toolbox-core/src/toolbox_core/sync_tool.py +++ b/packages/toolbox-core/src/toolbox_core/sync_tool.py @@ -152,9 +152,9 @@ def add_auth_token_getters( Raises: ValueError: If an auth source has already been registered either to - the tool or to the corresponding client. - """ + the tool or to the corresponding client. + """ new_async_tool = self.__async_tool.add_auth_token_getters(auth_token_getters) return ToolboxSyncTool(new_async_tool, self.__loop, self.__thread) @@ -175,7 +175,7 @@ def add_auth_token_getter( Raises: ValueError: If the auth source has already been registered either to - the tool or to the corresponding client. + the tool or to the corresponding client. """ return self.add_auth_token_getters({auth_source: get_id_token}) @@ -187,8 +187,8 @@ def bind_params( Binds parameters to values or callables that produce values. Args: - bound_params: A mapping of parameter names to values or callables that - produce values. + bound_params: A mapping of parameter names to values or callables + that produce values. Returns: A new ToolboxSyncTool instance with the specified parameters bound. @@ -198,7 +198,6 @@ def bind_params( tool's definition. """ - new_async_tool = self.__async_tool.bind_params(bound_params) return ToolboxSyncTool(new_async_tool, self.__loop, self.__thread)