feat(bedrock): configurable Bedrock read/connect timeout#2223
Open
go-faustino wants to merge 2 commits into
Open
feat(bedrock): configurable Bedrock read/connect timeout#2223go-faustino wants to merge 2 commits into
go-faustino wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
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/connectTimeoutfrom the ModelConfig CRD → Go translator/types → Python ADK model config. - Updates the Bedrock boto3 client creation to optionally attach a
botocore.config.Configonly 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.
go-faustino
force-pushed
the
feat/bedrock-configurable-read-timeout
branch
5 times, most recently
from
July 15, 2026 22:22
7c2e077 to
0353076
Compare
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
force-pushed
the
feat/bedrock-configurable-read-timeout
branch
from
July 17, 2026 09:26
0353076 to
f3c7785
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Python ADK's Bedrock client is created with
boto3.client("bedrock-runtime", ...)and no botocoreConfig, so it uses botocore's ~60s default read timeout. Long Converse completions — large tool-augmented turns, extended reasoning — are aborted mid-response with aReadTimeoutError, 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
ModelConfigCRD down to the boto3 client.Changes
_bedrock.py:_get_bedrock_clientnow acceptsread_timeout/connect_timeoutand builds abotocore.config.Configonly when at least one is set, so the default behavior is unchanged.KAgentBedrockLlmexposes the two fields and forwards them.types.py: theBedrockmodel config gainsread_timeout/connect_timeout, passed intoKAgentBedrockLlm.BedrockConfiggainsreadTimeout/connectTimeout(seconds,minimum: 1), mapped through the Goadk.Bedrocktype and the agent translator. CRD manifest and the Helmkagent-crdscopy regenerated withcontroller-gen.Test
Adds Python unit tests in
tests/unittests/models/test_bedrock.py:configpassed when timeouts are unset (default preserved),read_timeoutsetsConfig.read_timeout(connect left at botocore's default),read_timeout+connect_timeoutboth applied.Go:
go build ./api/... ./core/internal/controller/translator/...andgo vet ./api/...pass; deepcopy regenerated.Example