Skip to content

Commit 9eaacd5

Browse files
fix(lib): tighten TemporalACPConfig + concise comments + import order
- Add `model_validator(mode="after")` on `TemporalACPConfig` that rejects setting both `payload_codec` and `data_converter`, mirroring the `get_temporal_client` ambiguity guard at config-construction time (consistent with the existing `plugins` / `interceptors` validators). - Collapse multi-line comments in `worker.py` and `clients/temporal/utils.py` to single-line context — the PR description carries the long-form rationale. - Ruff import-order autofixes on touched files (resolves CI lint). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent d85e614 commit 9eaacd5

6 files changed

Lines changed: 25 additions & 25 deletions

File tree

src/agentex/lib/core/clients/temporal/temporal_client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from temporalio.client import Client, WorkflowExecutionStatus
88
from temporalio.common import RetryPolicy as TemporalRetryPolicy, WorkflowIDReusePolicy
99
from temporalio.service import RPCError, RPCStatusCode
10-
from temporalio.converter import DataConverter, PayloadCodec
10+
from temporalio.converter import PayloadCodec, DataConverter
1111

1212
from agentex.lib.utils.logging import make_logger
1313
from agentex.lib.utils.model_utils import BaseModel

src/agentex/lib/core/clients/temporal/utils.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from temporalio.client import Client, Plugin as ClientPlugin
77
from temporalio.worker import Interceptor
88
from temporalio.runtime import Runtime, TelemetryConfig, OpenTelemetryConfig
9-
from temporalio.converter import DataConverter, PayloadCodec
9+
from temporalio.converter import PayloadCodec, DataConverter
1010
from temporalio.contrib.pydantic import pydantic_data_converter
1111

1212
# class DateTimeJSONEncoder(AdvancedJSONEncoder):
@@ -118,17 +118,11 @@ async def get_temporal_client(
118118
"kwarg. Specifying both is ambiguous."
119119
)
120120

121-
# Check if OpenAI plugin is present - it needs to configure its own data converter
122121
# Lazy import to avoid pulling in opentelemetry.sdk for non-Temporal agents
123122
from temporalio.contrib.openai_agents import OpenAIAgentsPlugin
124123

125124
has_openai_plugin = any(isinstance(p, OpenAIAgentsPlugin) for p in (plugins or []))
126125

127-
# When the OpenAI plugin is present, its `_data_converter` transformer
128-
# builds a fresh DataConverter (without any codec) if none is supplied,
129-
# so a standalone `payload_codec` kwarg would be silently dropped and
130-
# payloads would land in Temporal in plain text. Guide the caller to
131-
# the working composition path instead.
132126
if has_openai_plugin and payload_codec is not None and data_converter is None:
133127
raise ValueError(
134128
"payload_codec passed as a kwarg alongside OpenAIAgentsPlugin would "
@@ -145,10 +139,6 @@ async def get_temporal_client(
145139
}
146140

147141
if data_converter is not None:
148-
# Caller supplied a pre-built converter. With the OpenAI plugin present
149-
# and `payload_converter_class=OpenAIPayloadConverter` (or subclass),
150-
# the plugin's `_data_converter` transformer passes it through intact,
151-
# preserving any payload_codec.
152142
connect_kwargs["data_converter"] = data_converter
153143
elif not has_openai_plugin:
154144
dc = pydantic_data_converter

src/agentex/lib/core/temporal/workers/worker.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,17 +107,11 @@ async def get_temporal_client(
107107
"kwarg. Specifying both is ambiguous."
108108
)
109109

110-
# Check if OpenAI plugin is present - it needs to configure its own data converter
111110
# Lazy import to avoid pulling in opentelemetry.sdk for non-Temporal agents
112111
from temporalio.contrib.openai_agents import OpenAIAgentsPlugin
113112

114113
has_openai_plugin = any(isinstance(p, OpenAIAgentsPlugin) for p in (plugins or []))
115114

116-
# When the OpenAI plugin is present, its `_data_converter` transformer
117-
# builds a fresh DataConverter (without any codec) if none is supplied,
118-
# so a standalone `payload_codec` kwarg would be silently dropped and
119-
# payloads would land in Temporal in plain text. Guide the caller to
120-
# the working composition path instead.
121115
if has_openai_plugin and payload_codec is not None and data_converter is None:
122116
raise ValueError(
123117
"payload_codec passed as a kwarg alongside OpenAIAgentsPlugin would "
@@ -134,10 +128,6 @@ async def get_temporal_client(
134128
}
135129

136130
if data_converter is not None:
137-
# Caller supplied a pre-built converter. With the OpenAI plugin present
138-
# and `payload_converter_class=OpenAIPayloadConverter` (or subclass),
139-
# the plugin's `_data_converter` transformer passes it through intact,
140-
# preserving any payload_codec.
141131
connect_kwargs["data_converter"] = data_converter
142132
elif not has_openai_plugin:
143133
dc = custom_data_converter

src/agentex/lib/sdk/fastacp/impl/temporal_acp.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from contextlib import asynccontextmanager
55

66
from fastapi import FastAPI
7-
from temporalio.converter import DataConverter, PayloadCodec
7+
from temporalio.converter import PayloadCodec, DataConverter
88

99
from agentex.protocol.acp import (
1010
SendEventParams,

src/agentex/lib/types/fastacp.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from typing import Any, Literal
44

5-
from pydantic import Field, BaseModel, field_validator
5+
from pydantic import Field, BaseModel, field_validator, model_validator
66

77
from agentex.lib.core.clients.temporal.utils import validate_client_plugins, validate_worker_interceptors
88

@@ -89,6 +89,16 @@ def validate_interceptors(cls, v: list[Any]) -> list[Any]:
8989
validate_worker_interceptors(v)
9090
return v
9191

92+
@model_validator(mode="after")
93+
def _validate_codec_and_data_converter_mutually_exclusive(self) -> "TemporalACPConfig":
94+
if self.payload_codec is not None and self.data_converter is not None:
95+
raise ValueError(
96+
"Pass payload_codec inside `data_converter` "
97+
"(DataConverter(..., payload_codec=...)) instead of as a separate "
98+
"field. Specifying both is ambiguous."
99+
)
100+
return self
101+
92102

93103
class AsyncBaseACPConfig(AsyncACPConfig):
94104
"""Configuration for AsyncBaseACP implementation

tests/lib/test_payload_codec.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
import pytest
77
from temporalio.client import Client, Plugin as ClientPlugin
88
from temporalio.converter import (
9-
DataConverter,
109
PayloadCodec,
10+
DataConverter,
1111
DefaultPayloadConverter,
1212
)
1313
from temporalio.contrib.pydantic import pydantic_data_converter
@@ -354,6 +354,16 @@ def test_config_accepts_data_converter(self):
354354
dc = DataConverter(payload_codec=_NoopCodec())
355355
assert TemporalACPConfig(data_converter=dc).data_converter is dc
356356

357+
def test_config_rejects_codec_and_data_converter_together(self):
358+
from pydantic import ValidationError
359+
360+
from agentex.lib.types.fastacp import TemporalACPConfig
361+
362+
codec = _NoopCodec()
363+
dc = DataConverter(payload_codec=codec)
364+
with pytest.raises(ValidationError, match="Pass payload_codec inside `data_converter`"):
365+
TemporalACPConfig(payload_codec=codec, data_converter=dc)
366+
357367
def test_fastacp_forwards_data_converter_from_config(self):
358368
from agentex.lib.types.fastacp import TemporalACPConfig
359369
from agentex.lib.sdk.fastacp.fastacp import FastACP

0 commit comments

Comments
 (0)