From 0a614640436794377b1e8dd0bde5f6296f838a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Faustino?= Date: Mon, 13 Jul 2026 10:25:18 +0100 Subject: [PATCH 1/2] feat(bedrock): configurable Bedrock read/connect timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Python ADK's Bedrock client is created via boto3 with no botocore Config, so it uses botocore's ~60s default read timeout. Long Converse completions (large tool-augmented turns, extended reasoning) are aborted with a ReadTimeoutError. Add optional read_timeout / connect_timeout (seconds), plumbed end to end: - _get_bedrock_client builds a botocore Config only when a timeout is set, otherwise keeping boto3 defaults. - KAgentBedrockLlm and the Bedrock config type expose read_timeout / connect_timeout. - BedrockConfig CRD gains readTimeout / connectTimeout, mapped through the Go adk.Bedrock type and the agent translator; CRD + Helm CRD regenerated. Adds Python unit tests covering unset (no Config), read-only, and read+connect timeouts. Signed-off-by: Gonçalo Faustino --- go/api/adk/types.go | 7 ++++++ .../crd/bases/kagent.dev_modelconfigs.yaml | 15 +++++++++++++ go/api/v1alpha2/modelconfig_types.go | 15 +++++++++++++ go/api/v1alpha2/zz_generated.deepcopy.go | 10 +++++++++ .../translator/agent/adk_api_translator.go | 2 ++ .../templates/kagent.dev_modelconfigs.yaml | 15 +++++++++++++ .../src/kagent/adk/models/_bedrock.py | 22 +++++++++++++++++++ .../kagent-adk/src/kagent/adk/types.py | 7 ++++++ .../tests/unittests/models/test_bedrock.py | 20 +++++++++++++++++ 9 files changed, 113 insertions(+) diff --git a/go/api/adk/types.go b/go/api/adk/types.go index bcc587cf4..7c5f0f619 100644 --- a/go/api/adk/types.go +++ b/go/api/adk/types.go @@ -263,6 +263,13 @@ type Bedrock struct { // "5m" (default) or "1h". See the v1alpha2.BedrockConfig CRD doc for the // cost/compatibility trade-offs of "1h". CacheTTL string `json:"cache_ttl,omitempty"` + // ReadTimeout is the Bedrock HTTP client read timeout in seconds. Overrides + // botocore's ~60s default, which otherwise aborts long completions with a + // ReadTimeoutError. Nil keeps botocore's default. + ReadTimeout *int `json:"read_timeout,omitempty"` + // ConnectTimeout is the Bedrock HTTP client connect timeout in seconds. Nil + // keeps botocore's default. + ConnectTimeout *int `json:"connect_timeout,omitempty"` } func (b *Bedrock) MarshalJSON() ([]byte, error) { diff --git a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml index c7a4163d6..a7df394fe 100644 --- a/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml +++ b/go/api/config/crd/bases/kagent.dev_modelconfigs.yaml @@ -503,6 +503,12 @@ spec: - 5m - 1h type: string + connectTimeout: + description: |- + ConnectTimeout is the Bedrock HTTP client connect timeout in seconds. + When unset, botocore's default is used. + minimum: 1 + type: integer promptCaching: default: false description: |- @@ -521,6 +527,15 @@ spec: See https://docs.aws.amazon.com/bedrock/latest/userguide/prompt-caching.html for the current list of supported models and minimum prefix sizes. type: boolean + readTimeout: + description: |- + ReadTimeout is the Bedrock HTTP client read timeout in seconds. The + underlying botocore client defaults to ~60s, which aborts long + completions (large tool-augmented turns, extended reasoning) with a + ReadTimeoutError. Raise this for agents that make long Converse calls. + When unset, botocore's default is used. + minimum: 1 + type: integer region: description: AWS region where the Bedrock model is available (e.g., us-east-1, us-west-2) diff --git a/go/api/v1alpha2/modelconfig_types.go b/go/api/v1alpha2/modelconfig_types.go index b46d07c81..9d5924c2c 100644 --- a/go/api/v1alpha2/modelconfig_types.go +++ b/go/api/v1alpha2/modelconfig_types.go @@ -294,6 +294,21 @@ type BedrockConfig struct { // +kubebuilder:validation:Enum="5m";"1h" // +kubebuilder:default="5m" CacheTTL string `json:"cacheTTL,omitempty"` + + // ReadTimeout is the Bedrock HTTP client read timeout in seconds. The + // underlying botocore client defaults to ~60s, which aborts long + // completions (large tool-augmented turns, extended reasoning) with a + // ReadTimeoutError. Raise this for agents that make long Converse calls. + // When unset, botocore's default is used. + // +optional + // +kubebuilder:validation:Minimum=1 + ReadTimeout *int `json:"readTimeout,omitempty"` + + // ConnectTimeout is the Bedrock HTTP client connect timeout in seconds. + // When unset, botocore's default is used. + // +optional + // +kubebuilder:validation:Minimum=1 + ConnectTimeout *int `json:"connectTimeout,omitempty"` } // SAPAICoreConfig contains SAP AI Core-specific configuration options. diff --git a/go/api/v1alpha2/zz_generated.deepcopy.go b/go/api/v1alpha2/zz_generated.deepcopy.go index 98d89ec39..15a9de59e 100644 --- a/go/api/v1alpha2/zz_generated.deepcopy.go +++ b/go/api/v1alpha2/zz_generated.deepcopy.go @@ -687,6 +687,16 @@ func (in *BedrockConfig) DeepCopyInto(out *BedrockConfig) { *out = new(apiextensionsv1.JSON) (*in).DeepCopyInto(*out) } + if in.ReadTimeout != nil { + in, out := &in.ReadTimeout, &out.ReadTimeout + *out = new(int) + **out = **in + } + if in.ConnectTimeout != nil { + in, out := &in.ConnectTimeout, &out.ConnectTimeout + *out = new(int) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BedrockConfig. diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index 1aba032f0..3235c839b 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -821,6 +821,8 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC AdditionalModelRequestFields: additionalFields, PromptCaching: model.Spec.Bedrock.PromptCaching, CacheTTL: model.Spec.Bedrock.CacheTTL, + ReadTimeout: model.Spec.Bedrock.ReadTimeout, + ConnectTimeout: model.Spec.Bedrock.ConnectTimeout, } // Populate TLS fields in BaseModel diff --git a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml index c7a4163d6..a7df394fe 100644 --- a/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml +++ b/helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml @@ -503,6 +503,12 @@ spec: - 5m - 1h type: string + connectTimeout: + description: |- + ConnectTimeout is the Bedrock HTTP client connect timeout in seconds. + When unset, botocore's default is used. + minimum: 1 + type: integer promptCaching: default: false description: |- @@ -521,6 +527,15 @@ spec: See https://docs.aws.amazon.com/bedrock/latest/userguide/prompt-caching.html for the current list of supported models and minimum prefix sizes. type: boolean + readTimeout: + description: |- + ReadTimeout is the Bedrock HTTP client read timeout in seconds. The + underlying botocore client defaults to ~60s, which aborts long + completions (large tool-augmented turns, extended reasoning) with a + ReadTimeoutError. Raise this for agents that make long Converse calls. + When unset, botocore's default is used. + minimum: 1 + type: integer region: description: AWS region where the Bedrock model is available (e.g., us-east-1, us-west-2) diff --git a/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py b/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py index 5323a7ada..3c3137085 100644 --- a/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py +++ b/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, Any, AsyncGenerator, Optional import boto3 +from botocore.config import Config as BotocoreConfig from google.adk.models import BaseLlm from google.adk.models.llm_response import LlmResponse from google.genai import types @@ -85,10 +86,24 @@ def _get_bedrock_client( tls_disable_verify: Optional[bool] = None, tls_ca_cert_path: Optional[str] = None, tls_disable_system_cas: Optional[bool] = None, + read_timeout: Optional[int] = None, + connect_timeout: Optional[int] = None, ): region = os.environ.get("AWS_DEFAULT_REGION") or os.environ.get("AWS_REGION") or "us-east-1" kwargs: dict[str, Any] = {"region_name": region} + # botocore defaults to a 60s read timeout, which aborts long Bedrock + # completions (large tool-augmented turns, extended reasoning) with a + # ReadTimeoutError. Allow callers to raise it. Only build a Config when an + # override is set so we keep botocore's defaults otherwise. + timeout_config: dict[str, Any] = {} + if read_timeout is not None: + timeout_config["read_timeout"] = read_timeout + if connect_timeout is not None: + timeout_config["connect_timeout"] = connect_timeout + if timeout_config: + kwargs["config"] = BotocoreConfig(**timeout_config) + if extra_headers: # boto3 doesn't support custom headers natively; log and ignore logger.warning("extra_headers are not supported for Bedrock models and will be ignored.") @@ -279,6 +294,11 @@ class KAgentBedrockLlm(KAgentTLSMixin, BaseLlm): # "5m"/None (Bedrock's default 5-minute cache) or "1h" (extended-TTL, # narrower model support and higher cache-write cost). See _cache_point_block. cache_ttl: Optional[str] = None + # Bedrock HTTP client timeouts in seconds. read_timeout overrides botocore's + # ~60s default, which otherwise aborts long completions with a + # ReadTimeoutError. None keeps botocore's defaults. + read_timeout: Optional[int] = None + connect_timeout: Optional[int] = None model_config = {"arbitrary_types_allowed": True} @@ -289,6 +309,8 @@ def _client(self): tls_disable_verify=self.tls_disable_verify, tls_ca_cert_path=self.tls_ca_cert_path, tls_disable_system_cas=self.tls_disable_system_cas, + read_timeout=self.read_timeout, + connect_timeout=self.connect_timeout, ) @classmethod diff --git a/python/packages/kagent-adk/src/kagent/adk/types.py b/python/packages/kagent-adk/src/kagent/adk/types.py index 867ac317d..3bfdbcdc3 100644 --- a/python/packages/kagent-adk/src/kagent/adk/types.py +++ b/python/packages/kagent-adk/src/kagent/adk/types.py @@ -327,6 +327,11 @@ class Bedrock(BaseLLM): # extended-TTL caching. "1h" is supported on fewer models and billed at a # higher cache-write rate, so it is not strictly better than "5m". cache_ttl: Literal["5m", "1h"] | None = None + # Bedrock HTTP client timeouts in seconds. read_timeout overrides botocore's + # ~60s default, which otherwise aborts long completions with a + # ReadTimeoutError. None keeps botocore's defaults. + read_timeout: int | None = None + connect_timeout: int | None = None type: Literal["bedrock"] @@ -709,6 +714,8 @@ def _create_llm_from_model_config(model_config: ModelUnion): additional_model_request_fields=model_config.additional_model_request_fields, prompt_caching=model_config.prompt_caching, cache_ttl=model_config.cache_ttl, + read_timeout=model_config.read_timeout, + connect_timeout=model_config.connect_timeout, **_transport_kwargs(model_config), ) if model_config.type == "sap_ai_core": diff --git a/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py b/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py index 08cbfad5b..dafc894ed 100644 --- a/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py +++ b/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py @@ -172,6 +172,26 @@ def test_defaults_to_us_east_1(self): _get_bedrock_client() assert mock_boto.call_args.kwargs["region_name"] == "us-east-1" + def test_no_config_when_timeouts_unset(self): + with mock.patch("kagent.adk.models._bedrock.boto3.client") as mock_boto: + _get_bedrock_client() + assert "config" not in mock_boto.call_args.kwargs + + def test_read_timeout_sets_botocore_config(self): + with mock.patch("kagent.adk.models._bedrock.boto3.client") as mock_boto: + _get_bedrock_client(read_timeout=1800) + config = mock_boto.call_args.kwargs["config"] + assert config.read_timeout == 1800 + # connect_timeout untouched -> botocore keeps its 60s default + assert config.connect_timeout == 60 + + def test_read_and_connect_timeout(self): + with mock.patch("kagent.adk.models._bedrock.boto3.client") as mock_boto: + _get_bedrock_client(read_timeout=900, connect_timeout=10) + config = mock_boto.call_args.kwargs["config"] + assert config.read_timeout == 900 + assert config.connect_timeout == 10 + class TestKAgentBedrockLlm: def test_default_construction(self): From 356d406314cfebb6255098f6704ceaf887c0bfb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Faustino?= Date: Mon, 13 Jul 2026 10:49:14 +0100 Subject: [PATCH 2/2] fix(bedrock): validate timeouts >=1 and de-brittle timeout test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the Bedrock read/connect timeout config: - Constrain read_timeout / connect_timeout to >= 1 on both the Python Bedrock config type and KAgentBedrockLlm (Field(ge=1)), matching the ModelConfig CRD (minimum: 1) so invalid values can't reach boto3. - Stop hard-coding botocore's 60s default in the timeout test; assert against botocore.config.Config().connect_timeout instead. - Add tests for accepted positive timeouts and rejected 0/negative values. Signed-off-by: Gonçalo Faustino --- .../src/kagent/adk/models/_bedrock.py | 8 +++++--- .../kagent-adk/src/kagent/adk/types.py | 7 ++++--- .../tests/unittests/models/test_bedrock.py | 20 +++++++++++++++++-- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py b/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py index 3c3137085..8d66c6edc 100644 --- a/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py +++ b/python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py @@ -18,6 +18,7 @@ import boto3 from botocore.config import Config as BotocoreConfig from google.adk.models import BaseLlm +from pydantic import Field from google.adk.models.llm_response import LlmResponse from google.genai import types @@ -296,9 +297,10 @@ class KAgentBedrockLlm(KAgentTLSMixin, BaseLlm): cache_ttl: Optional[str] = None # Bedrock HTTP client timeouts in seconds. read_timeout overrides botocore's # ~60s default, which otherwise aborts long completions with a - # ReadTimeoutError. None keeps botocore's defaults. - read_timeout: Optional[int] = None - connect_timeout: Optional[int] = None + # ReadTimeoutError. None keeps botocore's defaults. Constrained to >= 1 so + # invalid values can't reach the boto3 client (matches the CRD contract). + read_timeout: Optional[int] = Field(default=None, ge=1) + connect_timeout: Optional[int] = Field(default=None, ge=1) model_config = {"arbitrary_types_allowed": True} diff --git a/python/packages/kagent-adk/src/kagent/adk/types.py b/python/packages/kagent-adk/src/kagent/adk/types.py index 3bfdbcdc3..ddddd5b2c 100644 --- a/python/packages/kagent-adk/src/kagent/adk/types.py +++ b/python/packages/kagent-adk/src/kagent/adk/types.py @@ -329,9 +329,10 @@ class Bedrock(BaseLLM): cache_ttl: Literal["5m", "1h"] | None = None # Bedrock HTTP client timeouts in seconds. read_timeout overrides botocore's # ~60s default, which otherwise aborts long completions with a - # ReadTimeoutError. None keeps botocore's defaults. - read_timeout: int | None = None - connect_timeout: int | None = None + # ReadTimeoutError. None keeps botocore's defaults. Constrained to >= 1 to + # match the ModelConfig CRD (readTimeout/connectTimeout minimum: 1). + read_timeout: int | None = Field(default=None, ge=1) + connect_timeout: int | None = Field(default=None, ge=1) type: Literal["bedrock"] diff --git a/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py b/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py index dafc894ed..37c82ec57 100644 --- a/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py +++ b/python/packages/kagent-adk/tests/unittests/models/test_bedrock.py @@ -178,12 +178,15 @@ def test_no_config_when_timeouts_unset(self): assert "config" not in mock_boto.call_args.kwargs def test_read_timeout_sets_botocore_config(self): + from botocore.config import Config as BotocoreConfig + with mock.patch("kagent.adk.models._bedrock.boto3.client") as mock_boto: _get_bedrock_client(read_timeout=1800) config = mock_boto.call_args.kwargs["config"] assert config.read_timeout == 1800 - # connect_timeout untouched -> botocore keeps its 60s default - assert config.connect_timeout == 60 + # connect_timeout untouched -> botocore keeps its own default, + # whatever that is for the installed version (don't hard-code it). + assert config.connect_timeout == BotocoreConfig().connect_timeout def test_read_and_connect_timeout(self): with mock.patch("kagent.adk.models._bedrock.boto3.client") as mock_boto: @@ -202,6 +205,19 @@ def test_non_anthropic_model_accepted(self): llm = KAgentBedrockLlm(model="meta.llama3-8b-instruct-v1:0") assert llm.model == "meta.llama3-8b-instruct-v1:0" + def test_positive_timeouts_accepted(self): + llm = KAgentBedrockLlm(model="meta.llama3-8b-instruct-v1:0", read_timeout=1800, connect_timeout=10) + assert llm.read_timeout == 1800 + assert llm.connect_timeout == 10 + + @pytest.mark.parametrize("field", ["read_timeout", "connect_timeout"]) + @pytest.mark.parametrize("value", [0, -5]) + def test_non_positive_timeouts_rejected(self, field, value): + from pydantic import ValidationError + + with pytest.raises(ValidationError): + KAgentBedrockLlm(model="meta.llama3-8b-instruct-v1:0", **{field: value}) + @pytest.mark.asyncio async def test_generate_calls_converse(self): llm = KAgentBedrockLlm(model="us.anthropic.claude-sonnet-4-20250514-v1:0")