Skip to content

feat: Add read timeout and connect timeout for bedrock provider #1259

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

Merged
merged 6 commits into from
Mar 31, 2025

Conversation

Wh1isper
Copy link
Contributor

Boto3 does not support configuring connect_timeout and read_timeout via environment variables

the kwargs' names are from: https://github.com/microsoft/autogen/pull/5529/files

This will simplify the user's usage and will not result in errors during long tool calls by default.

@Kludex
Copy link
Member

Kludex commented Mar 31, 2025

Is there any other parameter that will be included? I'm not sure what's the point that we stop adding parameters, because you can also just pass the bedrock client itself... 🤔

What do you think?

@Kludex Kludex self-assigned this Mar 31, 2025
@Wh1isper
Copy link
Contributor Author

Wh1isper commented Mar 31, 2025

@Kludex I agree with your concern. On one hand, the current variables should be sufficient, and on the other hand, if a variable can be supported by an environment variable (e.g. AWS_PROFILE for profile file), we don't need to add it.

The most painful thing in this case is that we can only pass the bedrock client as they are not supported via environment variable, if I allow user to pass in a model name, I have to manually patch the infer_model method...

Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Let's add those. :)

@Kludex Kludex merged commit c977484 into pydantic:main Mar 31, 2025
16 checks passed
@Wh1isper Wh1isper deleted the feat-add-timeout-in-bedrock-provider branch March 31, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants