chore: address feedback

Author: kurtisvg

packages/toolbox-core/src/toolbox_core/client.py

@@ -11,6 +11,7 @@
 # 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.
+import re
 import types
 from typing import Any, Callable, Optional
 
@@ -79,8 +80,9 @@ def __parse_tool(
             name=name,
             desc=schema.description,
             params=[p.to_param() for p in params],
+            # create a read-only values for the maps to prevent mutation
             required_authn_params=types.MappingProxyType(authn_params),
-            auth_service_token_getters=auth_token_getters,
+            auth_service_token_getters=types.MappingProxyType(auth_token_getters),
         )
         return tool
 

packages/toolbox-core/src/toolbox_core/tool.py

@@ -56,9 +56,9 @@ def __init__(
             desc: The description of the remote tool (used as its docstring).
             params: A list of `inspect.Parameter` objects defining the tool's
                 arguments and their types/defaults.
-            required_authn_params: A dict of required authenticated parameters that
-                need a auth_service_token_getter set for them yet.
-            auth_service_tokens: A dict of authService -> token (or callables that
+            required_authn_params: A dict of required authenticated parameters to a list
+                of services that provide values for them.
+            auth_service_token_getters: A dict of authService -> token (or callables that
                 produce a token)
         """
 
@@ -108,15 +108,19 @@ def __copy(
                 that produce a token)
 
         """
+        check = lambda val, default: val if val is not None else default
         return ToolboxTool(
-            session=session or self.__session,
-            base_url=base_url or self.__base_url,
-            name=name or self.__name__,
-            desc=desc or self.__desc,
-            params=params or self.__params,
-            required_authn_params=required_authn_params or self.__required_authn_params,
-            auth_service_token_getters=auth_service_token_getters
-            or self.__auth_service_token_getters,
+            session=check(session, self.__session),
+            base_url=check(base_url, self.__base_url),
+            name=check(name, self.__name__),
+            desc=check(desc, self.__desc),
+            params=check(params, self.__params),
+            required_authn_params=check(
+                required_authn_params, self.__required_authn_params
+            ),
+            auth_service_token_getters=check(
+                auth_service_token_getters, self.__auth_service_token_getters
+            ),
         )
 
     async def __call__(self, *args: Any, **kwargs: Any) -> str:
@@ -138,7 +142,7 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str:
         if len(self.__required_authn_params) > 0:
             req_auth_services = set(l for l in self.__required_authn_params.keys())
             raise Exception(
-                f"One of more of the following authn services are required to invoke this tool: {','.join(req_auth_services)}"
+                f"One or more of the following authn services are required to invoke this tool: {','.join(req_auth_services)}"
             )
 
         # validate inputs to this call using the signature
@@ -167,7 +171,7 @@ def add_auth_token_getters(
         auth_token_getters: Mapping[str, Callable[[], str]],
     ) -> "ToolboxTool":
         """
-        Registers a auth token getter function that is used for AuthServices when tools
+        Registers an auth token getter function that is used for AuthServices when tools
         are invoked.
 
         Args:
@@ -204,25 +208,27 @@ def add_auth_token_getters(
 
 
 def filter_required_authn_params(
-    req_authn_params: Mapping[str, list[str]], auth_services: Iterable[str]
+    req_authn_params: Mapping[str, list[str]], auth_service_names: Iterable[str]
 ) -> dict[str, list[str]]:
     """
-    Utility function for reducing 'req_authn_params' to a subset of parameters that aren't supplied by a least one service in auth_services.
+    Utility function for reducing 'req_authn_params' to a subset of parameters that
+    aren't supplied by at least one service in auth_services.
 
     Args:
         req_authn_params: A mapping of parameter names to sets of required
             authentication services.
-        auth_services: An iterable of authentication service names for which
+        auth_service_names: An iterable of authentication service names for which
             token getters are available.
 
     Returns:
         A new dictionary representing the subset of required authentication
-        parameters that are not covered by the provided `auth_services`.
+        parameters that are not covered by the provided `auth_service_names`.
     """
-    req_params = {}
+    required_params = {}  # params that are still required with provided auth_services
     for param, services in req_authn_params.items():
-        # if we don't have a token_getter for any of the services required by the param, the param is still required
-        required = not any(s in services for s in auth_services)
+        # if we don't have a token_getter for any of the services required by the param,
+        # the param is still required
+        required = not any(s in services for s in auth_service_names)
         if required:
-            req_params[param] = services
-    return req_params
+            required_params[param] = services
+    return required_params