-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Bedrock: workaround LiteLLM passthrough issues #6778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,6 +226,11 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH | |
| // Use API key/token-based authentication if enabled and API key is set | ||
| clientConfig.token = { token: this.options.awsApiKey } | ||
| clientConfig.authSchemePreference = ["httpBearerAuth"] // Otherwise there's no end of credential problems. | ||
| clientConfig.requestHandler = { | ||
| // This should be the default anyway, but without setting something | ||
| // this provider fails to work with LiteLLM passthrough. | ||
| requestTimeout: 0, | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a workaround for a specific LiteLLM passthrough issue, consider adding a test case that verifies the Also, would it make sense to add a configuration option like |
||
| } else if (this.options.awsUseProfile && this.options.awsProfile) { | ||
| // Use profile-based credentials if enabled and profile is set | ||
| clientConfig.credentials = fromIni({ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment mentions "This should be the default anyway" but it's unclear why explicitly setting
requestTimeout: 0fixes the LiteLLM issue. Could you provide more context about what was happening without this setting? Was LiteLLM timing out immediately, or was there a different default being applied?Also, setting
requestTimeout: 0means no timeout limit. While this fixes the immediate issue, could this potentially cause requests to hang indefinitely in certain failure scenarios? Would a reasonable timeout value (e.g., 300000 for 5 minutes) work instead?