feat: configurable keep alive duration in network config - #5328
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (35)
📝 WalkthroughWalkthroughAdds a configurable pooled-connection keep-alive timeout to backend and UI network configuration, persists and validates it, and applies it across provider HTTP clients instead of fixed idle durations. ChangesKeep-alive configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant NetworkFormFragment
participant ProviderConfig
participant ProviderClient
NetworkFormFragment->>ProviderConfig: submit keep_alive_timeout_in_seconds
ProviderConfig->>ProviderConfig: marshal and apply defaults
ProviderConfig->>ProviderClient: initialize idle duration from KeepAliveTimeoutInSeconds
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "feat: configurable keep alive duration i..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/schemas/provider.go`:
- Around line 585-587: Update the KeepAliveTimeoutInSeconds validation in the
ProviderConfig normalization path to replace values outside the documented
1–3600-second range, including values above 3600, with the existing default.
Reuse the configuration schema’s shared maximum constant if available rather
than hardcoding a duplicate limit, while preserving valid values unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ea923256-cfbe-4526-b386-33d35e5a579d
📒 Files selected for processing (34)
core/providers/anthropic/anthropic.gocore/providers/azure/azure.gocore/providers/bedrock/bedrock.gocore/providers/bedrockmantle/bedrockmantle.gocore/providers/cerebras/cerebras.gocore/providers/cohere/cohere.gocore/providers/deepseek/deepseek.gocore/providers/elevenlabs/elevenlabs.gocore/providers/fireworks/fireworks.gocore/providers/gemini/gemini.gocore/providers/groq/groq.gocore/providers/huggingface/huggingface.gocore/providers/mistral/mistral.gocore/providers/nebius/nebius.gocore/providers/ollama/ollama.gocore/providers/openai/openai.gocore/providers/opencode/opencode.gocore/providers/openrouter/openrouter.gocore/providers/parasail/parasail.gocore/providers/perplexity/perplexity.gocore/providers/replicate/replicate.gocore/providers/runware/runware.gocore/providers/runway/runway.gocore/providers/sarvam/sarvam.gocore/providers/sgl/sgl.gocore/providers/vertex/vertex.gocore/providers/vllm/vllm.gocore/providers/xai/xai.gocore/schemas/provider.gotransports/config.schema.jsonui/app/workspace/providers/fragments/networkFormFragment.tsxui/lib/constants/config.tsui/lib/schemas/providerForm.tsui/lib/types/config.ts
818ffb1 to
343680e
Compare
Merge activity
|
The base branch was changed.
343680e to
4fbaea3
Compare
4fbaea3 to
e61fa61
Compare
## Summary The idle connection keep-alive timeout for all provider HTTP clients was previously hardcoded (30s for most providers, 60s for Runware and Runway). This PR replaces those hardcoded values with a configurable `keep_alive_timeout_in_seconds` field on `NetworkConfig`, defaulting to 30 seconds. This allows operators to tune the idle connection lifetime to stay below the upstream server's own keep-alive timeout, preventing Bifrost from attempting to reuse connections that the server has already closed. ## Changes - Added `KeepAliveTimeoutInSeconds` to `NetworkConfig` with a default of 30 seconds, validation in `CheckAndSetDefaults`, and full JSON marshal/unmarshal support. - Replaced all hardcoded `MaxIdleConnDuration` / `IdleConnTimeout` values across every provider (including the previously special-cased 60s values for Runware and Runway) with `time.Second * time.Duration(config.NetworkConfig.KeepAliveTimeoutInSeconds)`. - Added `keep_alive_timeout_in_seconds` to the JSON config schema with `minimum: 1` and `maximum: 3600`. - Added a `Keep-Alive Timeout` input field to the provider network configuration form in the UI, with validation (1–3600), human-readable duration display, and a default of 30. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` Set `keep_alive_timeout_in_seconds` in a provider's `network_config` and confirm the value is respected at runtime. Omitting the field should fall back to the 30-second default. Setting it to `0` or a negative value should also fall back to the default via `CheckAndSetDefaults`. ## Breaking changes - [ ] Yes - [x] No The default value (30s) matches the previous hardcoded value for all providers except Runware and Runway, which previously used 60s. Those two providers will now default to 30s unless `keep_alive_timeout_in_seconds: 60` is explicitly set in their config. ## Related issues ## Security considerations None. This change only affects connection pool lifecycle timing and introduces no new secrets, auth paths, or PII handling. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary The idle connection keep-alive timeout for all provider HTTP clients was previously hardcoded (30s for most providers, 60s for Runware and Runway). This PR replaces those hardcoded values with a configurable `keep_alive_timeout_in_seconds` field on `NetworkConfig`, defaulting to 30 seconds. This allows operators to tune the idle connection lifetime to stay below the upstream server's own keep-alive timeout, preventing Bifrost from attempting to reuse connections that the server has already closed. ## Changes - Added `KeepAliveTimeoutInSeconds` to `NetworkConfig` with a default of 30 seconds, validation in `CheckAndSetDefaults`, and full JSON marshal/unmarshal support. - Replaced all hardcoded `MaxIdleConnDuration` / `IdleConnTimeout` values across every provider (including the previously special-cased 60s values for Runware and Runway) with `time.Second * time.Duration(config.NetworkConfig.KeepAliveTimeoutInSeconds)`. - Added `keep_alive_timeout_in_seconds` to the JSON config schema with `minimum: 1` and `maximum: 3600`. - Added a `Keep-Alive Timeout` input field to the provider network configuration form in the UI, with validation (1–3600), human-readable duration display, and a default of 30. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` Set `keep_alive_timeout_in_seconds` in a provider's `network_config` and confirm the value is respected at runtime. Omitting the field should fall back to the 30-second default. Setting it to `0` or a negative value should also fall back to the default via `CheckAndSetDefaults`. ## Breaking changes - [ ] Yes - [x] No The default value (30s) matches the previous hardcoded value for all providers except Runware and Runway, which previously used 60s. Those two providers will now default to 30s unless `keep_alive_timeout_in_seconds: 60` is explicitly set in their config. ## Related issues ## Security considerations None. This change only affects connection pool lifecycle timing and introduces no new secrets, auth paths, or PII handling. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
The idle connection keep-alive timeout for all provider HTTP clients was previously hardcoded (30s for most providers, 60s for Runware and Runway). This PR replaces those hardcoded values with a configurable
keep_alive_timeout_in_secondsfield onNetworkConfig, defaulting to 30 seconds. This allows operators to tune the idle connection lifetime to stay below the upstream server's own keep-alive timeout, preventing Bifrost from attempting to reuse connections that the server has already closed.Changes
KeepAliveTimeoutInSecondstoNetworkConfigwith a default of 30 seconds, validation inCheckAndSetDefaults, and full JSON marshal/unmarshal support.MaxIdleConnDuration/IdleConnTimeoutvalues across every provider (including the previously special-cased 60s values for Runware and Runway) withtime.Second * time.Duration(config.NetworkConfig.KeepAliveTimeoutInSeconds).keep_alive_timeout_in_secondsto the JSON config schema withminimum: 1andmaximum: 3600.Keep-Alive Timeoutinput field to the provider network configuration form in the UI, with validation (1–3600), human-readable duration display, and a default of 30.Type of change
Affected areas
How to test
Set
keep_alive_timeout_in_secondsin a provider'snetwork_configand confirm the value is respected at runtime. Omitting the field should fall back to the 30-second default. Setting it to0or a negative value should also fall back to the default viaCheckAndSetDefaults.Breaking changes
The default value (30s) matches the previous hardcoded value for all providers except Runware and Runway, which previously used 60s. Those two providers will now default to 30s unless
keep_alive_timeout_in_seconds: 60is explicitly set in their config.Related issues
Security considerations
None. This change only affects connection pool lifecycle timing and introduces no new secrets, auth paths, or PII handling.
Checklist
docs/contributing/README.mdand followed the guidelines