tackle feedback

Signed-off-by: Filinto Duran <[email protected]>

Author: filintod

durabletask/aio/internal/shared.py

@@ -1,7 +1,7 @@
 # Copyright (c) The Dapr Authors.
 # Licensed under the MIT License.
 
-from typing import Dict, Optional, Sequence, Union
+from typing import Optional, Sequence, Union
 
 import grpc
 from grpc import aio as grpc_aio
@@ -51,19 +51,16 @@ def get_grpc_aio_channel(
             host_address = host_address[len(protocol) :]
             break
 
-    # channel interceptors/options
-    channel_kwargs: Dict[str, ChannelArgumentType | Sequence[ClientInterceptor]] = dict(
-        interceptors=interceptors
-    )
     if options is not None:
         validate_grpc_options(options)
-        channel_kwargs["options"] = options
 
     if secure_channel:
         channel = grpc_aio.secure_channel(
-            host_address, grpc.ssl_channel_credentials(), **channel_kwargs
+            host_address, grpc.ssl_channel_credentials(), interceptors=interceptors, options=options
         )
     else:
-        channel = grpc_aio.insecure_channel(host_address, **channel_kwargs)
+        channel = grpc_aio.insecure_channel(
+            host_address, interceptors=interceptors, options=options
+        )
 
     return channel

durabletask/internal/shared.py

@@ -97,17 +97,11 @@ def get_grpc_channel(
     if options is not None:
         # validate all options keys prefix starts with `grpc.`
         validate_grpc_options(options)
-        if secure_channel:
-            channel = grpc.secure_channel(
-                host_address, grpc.ssl_channel_credentials(), options=options
-            )
-        else:
-            channel = grpc.insecure_channel(host_address, options=options)
+
+    if secure_channel:
+        channel = grpc.secure_channel(host_address, grpc.ssl_channel_credentials(), options=options)
     else:
-        if secure_channel:
-            channel = grpc.secure_channel(host_address, grpc.ssl_channel_credentials())
-        else:
-            channel = grpc.insecure_channel(host_address)
+        channel = grpc.insecure_channel(host_address, options=options)
 
     # Apply interceptors ONLY if they exist
     if interceptors:

tests/durabletask/test_client.py

@@ -1,4 +1,4 @@
-from unittest.mock import ANY, patch
+from unittest.mock import patch
 
 from durabletask.internal.grpc_interceptor import DefaultClientInterceptorImpl
 from durabletask.internal.shared import get_default_host_address, get_grpc_channel
@@ -11,7 +11,9 @@
 def test_get_grpc_channel_insecure():
     with patch("grpc.insecure_channel") as mock_channel:
         get_grpc_channel(HOST_ADDRESS, False, interceptors=INTERCEPTORS)
-        mock_channel.assert_called_once_with(HOST_ADDRESS)
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_channel_secure():
@@ -20,13 +22,18 @@ def test_get_grpc_channel_secure():
         patch("grpc.ssl_channel_credentials") as mock_credentials,
     ):
         get_grpc_channel(HOST_ADDRESS, True, interceptors=INTERCEPTORS)
-        mock_channel.assert_called_once_with(HOST_ADDRESS, mock_credentials.return_value)
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert args[1] == mock_credentials.return_value
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_channel_default_host_address():
     with patch("grpc.insecure_channel") as mock_channel:
         get_grpc_channel(None, False, interceptors=INTERCEPTORS)
-        mock_channel.assert_called_once_with(get_default_host_address())
+        args, kwargs = mock_channel.call_args
+        assert args[0] == get_default_host_address()
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_channel_with_metadata():
@@ -35,7 +42,9 @@ def test_get_grpc_channel_with_metadata():
         patch("grpc.intercept_channel") as mock_intercept_channel,
     ):
         get_grpc_channel(HOST_ADDRESS, False, interceptors=INTERCEPTORS)
-        mock_channel.assert_called_once_with(HOST_ADDRESS)
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert "options" in kwargs and kwargs["options"] is None
         mock_intercept_channel.assert_called_once()
 
         # Capture and check the arguments passed to intercept_channel()
@@ -54,40 +63,60 @@ def test_grpc_channel_with_host_name_protocol_stripping():
 
         prefix = "grpc://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_insecure_channel.assert_called_with(host_name)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "http://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_insecure_channel.assert_called_with(host_name)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "HTTP://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_insecure_channel.assert_called_with(host_name)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "GRPC://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_insecure_channel.assert_called_with(host_name)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = ""
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_insecure_channel.assert_called_with(host_name)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "grpcs://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_secure_channel.assert_called_with(host_name, ANY)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "https://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_secure_channel.assert_called_with(host_name, ANY)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "HTTPS://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_secure_channel.assert_called_with(host_name, ANY)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "GRPCS://"
         get_grpc_channel(prefix + host_name, interceptors=INTERCEPTORS)
-        mock_secure_channel.assert_called_with(host_name, ANY)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = ""
         get_grpc_channel(prefix + host_name, True, interceptors=INTERCEPTORS)
-        mock_secure_channel.assert_called_with(host_name, ANY)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert "options" in kwargs and kwargs["options"] is None

tests/durabletask/test_client_async.py

@@ -1,7 +1,7 @@
 # Copyright (c) The Dapr Authors.
 # Licensed under the MIT License.
 
-from unittest.mock import ANY, patch
+from unittest.mock import patch
 
 from durabletask.aio.client import AsyncTaskHubGrpcClient
 from durabletask.aio.internal.grpc_interceptor import DefaultClientInterceptorImpl
@@ -16,7 +16,10 @@
 def test_get_grpc_aio_channel_insecure():
     with patch("durabletask.aio.internal.shared.grpc_aio.insecure_channel") as mock_channel:
         get_grpc_aio_channel(HOST_ADDRESS, False, interceptors=INTERCEPTORS_AIO)
-        mock_channel.assert_called_once_with(HOST_ADDRESS, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_aio_channel_secure():
@@ -25,23 +28,29 @@ def test_get_grpc_aio_channel_secure():
         patch("grpc.ssl_channel_credentials") as mock_credentials,
     ):
         get_grpc_aio_channel(HOST_ADDRESS, True, interceptors=INTERCEPTORS_AIO)
-        mock_channel.assert_called_once_with(
-            HOST_ADDRESS, mock_credentials.return_value, interceptors=INTERCEPTORS_AIO
-        )
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert args[1] == mock_credentials.return_value
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_aio_channel_default_host_address():
     with patch("durabletask.aio.internal.shared.grpc_aio.insecure_channel") as mock_channel:
         get_grpc_aio_channel(None, False, interceptors=INTERCEPTORS_AIO)
-        mock_channel.assert_called_once_with(
-            get_default_host_address(), interceptors=INTERCEPTORS_AIO
-        )
+        args, kwargs = mock_channel.call_args
+        assert args[0] == get_default_host_address()
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_get_grpc_aio_channel_with_interceptors():
     with patch("durabletask.aio.internal.shared.grpc_aio.insecure_channel") as mock_channel:
         get_grpc_aio_channel(HOST_ADDRESS, False, interceptors=INTERCEPTORS_AIO)
-        mock_channel.assert_called_once_with(HOST_ADDRESS, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_channel.call_args
+        assert args[0] == HOST_ADDRESS
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         # Capture and check the arguments passed to insecure_channel()
         args, kwargs = mock_channel.call_args
@@ -61,43 +70,73 @@ def test_grpc_aio_channel_with_host_name_protocol_stripping():
 
         prefix = "grpc://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_insecure_channel.assert_called_with(host_name, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "http://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_insecure_channel.assert_called_with(host_name, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "HTTP://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_insecure_channel.assert_called_with(host_name, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "GRPC://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_insecure_channel.assert_called_with(host_name, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = ""
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_insecure_channel.assert_called_with(host_name, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_insecure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "grpcs://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_secure_channel.assert_called_with(host_name, ANY, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "https://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_secure_channel.assert_called_with(host_name, ANY, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "HTTPS://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_secure_channel.assert_called_with(host_name, ANY, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = "GRPCS://"
         get_grpc_aio_channel(prefix + host_name, interceptors=INTERCEPTORS_AIO)
-        mock_secure_channel.assert_called_with(host_name, ANY, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
         prefix = ""
         get_grpc_aio_channel(prefix + host_name, True, interceptors=INTERCEPTORS_AIO)
-        mock_secure_channel.assert_called_with(host_name, ANY, interceptors=INTERCEPTORS_AIO)
+        args, kwargs = mock_secure_channel.call_args
+        assert args[0] == host_name
+        assert kwargs.get("interceptors") == INTERCEPTORS_AIO
+        assert "options" in kwargs and kwargs["options"] is None
 
 
 def test_async_client_construct_with_metadata():