Skip to content

feat(bedrock): configurable Bedrock read/connect timeout#2223

Open
go-faustino wants to merge 2 commits into
kagent-dev:mainfrom
go-faustino:feat/bedrock-configurable-read-timeout
Open

feat(bedrock): configurable Bedrock read/connect timeout#2223
go-faustino wants to merge 2 commits into
kagent-dev:mainfrom
go-faustino:feat/bedrock-configurable-read-timeout

Conversation

@go-faustino

Copy link
Copy Markdown
Contributor

Summary

The Python ADK's Bedrock client is created with boto3.client("bedrock-runtime", ...) and no botocore Config, so it uses botocore's ~60s default read timeout. Long Converse completions — large tool-augmented turns, extended reasoning — are aborted mid-response with a ReadTimeoutError, surfaced to the user as an API error.

This adds an optional, configurable Bedrock read timeout (and connect timeout), plumbed end to end from the ModelConfig CRD down to the boto3 client.

Changes

  • _bedrock.py: _get_bedrock_client now accepts read_timeout / connect_timeout and builds a botocore.config.Config only when at least one is set, so the default behavior is unchanged. KAgentBedrockLlm exposes the two fields and forwards them.
  • types.py: the Bedrock model config gains read_timeout / connect_timeout, passed into KAgentBedrockLlm.
  • CRD: BedrockConfig gains readTimeout / connectTimeout (seconds, minimum: 1), mapped through the Go adk.Bedrock type and the agent translator. CRD manifest and the Helm kagent-crds copy regenerated with controller-gen.

Test

Adds Python unit tests in tests/unittests/models/test_bedrock.py:

  • no config passed when timeouts are unset (default preserved),
  • read_timeout sets Config.read_timeout (connect left at botocore's default),
  • read_timeout + connect_timeout both applied.
uv run pytest tests/unittests/models/test_bedrock.py -q
36 passed

Go: go build ./api/... ./core/internal/controller/translator/... and go vet ./api/... pass; deepcopy regenerated.

Example

apiVersion: kagent.dev/v1alpha2
kind: ModelConfig
spec:
  provider: Bedrock
  model: us.anthropic.claude-sonnet-4-20250514-v1:0
  bedrock:
    region: us-east-1
    readTimeout: 1800   # 30 minutes

@go-faustino
go-faustino requested a review from a team as a code owner July 13, 2026 09:25
Copilot AI review requested due to automatic review settings July 13, 2026 09:25
@github-actions github-actions Bot added the enhancement New feature or request label Jul 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end configuration for AWS Bedrock client HTTP connect/read timeouts so long Converse responses aren’t aborted by botocore’s ~60s default read timeout, while preserving existing default behavior when timeouts are unset.

Changes:

  • Plumbs readTimeout / connectTimeout from the ModelConfig CRD → Go translator/types → Python ADK model config.
  • Updates the Bedrock boto3 client creation to optionally attach a botocore.config.Config only when a timeout override is provided.
  • Adds Python unit tests covering the “no config when unset” behavior and the timeout override cases.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
python/packages/kagent-adk/tests/unittests/models/test_bedrock.py Adds unit tests asserting boto3 client config behavior for unset/set timeouts.
python/packages/kagent-adk/src/kagent/adk/types.py Extends the Python Bedrock model config schema to include timeout fields and forwards them into Bedrock LLM construction.
python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py Adds optional read_timeout/connect_timeout plumbing into the Bedrock boto3 client via botocore.config.Config.
helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml Regenerates Helm CRD template to include Bedrock timeout fields with minimum validation.
go/core/internal/controller/translator/agent/adk_api_translator.go Maps Bedrock timeout fields from CRD spec into the ADK Bedrock payload.
go/api/v1alpha2/zz_generated.deepcopy.go Regenerates deepcopy for new BedrockConfig fields.
go/api/v1alpha2/modelconfig_types.go Adds BedrockConfig readTimeout/connectTimeout with Minimum=1 validation and docs.
go/api/config/crd/bases/kagent.dev_modelconfigs.yaml Updates the base CRD manifest with Bedrock timeout schema/docs.
go/api/adk/types.go Extends Go ADK Bedrock type to include timeout fields for JSON payloads.
Files not reviewed (1)
  • go/api/v1alpha2/zz_generated.deepcopy.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/packages/kagent-adk/tests/unittests/models/test_bedrock.py Outdated
Comment thread python/packages/kagent-adk/src/kagent/adk/types.py Outdated
Comment thread python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py
@go-faustino
go-faustino force-pushed the feat/bedrock-configurable-read-timeout branch 5 times, most recently from 7c2e077 to 0353076 Compare July 15, 2026 22:22
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 <goncalo.santos@wellhub.com>
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 <goncalo.santos@wellhub.com>
@go-faustino
go-faustino force-pushed the feat/bedrock-configurable-read-timeout branch from 0353076 to f3c7785 Compare July 17, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants