Skip to content

Commit b0b9d3e

Browse files
committed
Address logger review findings
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3b014e3-808e-455f-b932-01d0a7aa5c93
1 parent 0942fc1 commit b0b9d3e

3 files changed

Lines changed: 50 additions & 3 deletions

File tree

‎durabletask/client.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ def __init__(self, *,
423423
data_converter: DataConverter | None = None,
424424
emit_trace_spans: bool = True):
425425

426+
self._logger = shared.get_logger("client", log_handler, log_formatter, logger)
426427
self._owns_channel = channel is None
427428
self._data_converter = data_converter if data_converter is not None else JsonDataConverter()
428429
self._host_address = (
@@ -483,7 +484,6 @@ def __init__(self, *,
483484
# can prepend the interceptor themselves via grpc.intercept_channel.
484485
self._channel = channel
485486
self._stub = cast(_SyncTaskHubSidecarServiceStub, stubs.TaskHubSidecarServiceStub(channel))
486-
self._logger = shared.get_logger("client", log_handler, log_formatter, logger)
487487
self.default_version = default_version
488488
self._payload_store = payload_store
489489
self._emit_trace_spans = emit_trace_spans
@@ -953,6 +953,7 @@ def __init__(self, *,
953953
data_converter: DataConverter | None = None,
954954
emit_trace_spans: bool = True):
955955

956+
self._logger = shared.get_logger("async_client", log_handler, log_formatter, logger)
956957
self._owns_channel = channel is None
957958
self._data_converter = data_converter if data_converter is not None else JsonDataConverter()
958959
self._host_address = (
@@ -1010,7 +1011,6 @@ def __init__(self, *,
10101011
if channel is not None
10111012
else None
10121013
)
1013-
self._logger = shared.get_logger("async_client", log_handler, log_formatter, logger)
10141014
self.default_version = default_version
10151015
self._payload_store = payload_store
10161016
self._emit_trace_spans = emit_trace_spans

‎durabletask/internal/shared.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def _get_legacy_logging_warning_stacklevel() -> int:
8181

8282
while frame is not None:
8383
module_name = frame.f_globals.get("__name__", "")
84-
if not module_name.startswith("durabletask"):
84+
if module_name != "durabletask" and not module_name.startswith("durabletask."):
8585
break
8686
stacklevel += 1
8787
frame = frame.f_back

‎tests/durabletask/test_logging.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import pytest
88

99
from durabletask.client import AsyncTaskHubGrpcClient, TaskHubGrpcClient
10+
from durabletask.internal.shared import get_logger
1011
from durabletask.worker import TaskHubGrpcWorker
1112

1213

@@ -55,3 +56,49 @@ def test_logger_cannot_be_combined_with_legacy_logging_options():
5556
logger=logging.Logger("test.durabletask"),
5657
log_handler=logging.NullHandler(),
5758
)
59+
60+
61+
def test_logger_conflict_does_not_create_a_sync_grpc_channel(monkeypatch):
62+
def unexpected_channel_creation(*args, **kwargs):
63+
raise AssertionError("A gRPC channel should not be created for invalid logging options.")
64+
65+
monkeypatch.setattr("durabletask.client.shared.get_grpc_channel", unexpected_channel_creation)
66+
67+
with pytest.raises(ValueError, match="cannot be combined"):
68+
TaskHubGrpcClient(
69+
logger=logging.Logger("test.durabletask"),
70+
log_handler=logging.NullHandler(),
71+
)
72+
73+
74+
def test_logger_conflict_does_not_create_an_async_grpc_channel(monkeypatch):
75+
def unexpected_channel_creation(*args, **kwargs):
76+
raise AssertionError("A gRPC channel should not be created for invalid logging options.")
77+
78+
monkeypatch.setattr("durabletask.client.shared.get_async_grpc_channel", unexpected_channel_creation)
79+
80+
with pytest.raises(ValueError, match="cannot be combined"):
81+
AsyncTaskHubGrpcClient(
82+
logger=logging.Logger("test.durabletask"),
83+
log_handler=logging.NullHandler(),
84+
)
85+
86+
87+
def test_legacy_logging_warning_does_not_skip_similarly_named_application_module():
88+
namespace = {
89+
"__name__": "durabletask_app",
90+
"get_logger": get_logger,
91+
"logging": logging,
92+
}
93+
exec(
94+
"def create_logger():\n"
95+
" return get_logger('test', log_handler=logging.NullHandler())\n",
96+
namespace,
97+
)
98+
99+
with pytest.warns(DeprecationWarning, match="log_handler") as warnings:
100+
namespace["create_logger"]()
101+
102+
warning = next(warning for warning in warnings if "log_handler" in str(warning.message))
103+
assert warning.filename == "<string>"
104+
assert warning.lineno == 2

0 commit comments

Comments
 (0)