-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Make BedrockAwsConnectionProperties#region
empty by default
#3225
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
base: main
Are you sure you want to change the base?
Conversation
@therepanic Thanks for the PR! Could you update the documentation for the change (removing the default)? |
Thanks, @ilayaperumalg. Sorry, I accidentally formatted the wrong changes and closed the PR. I changed the documentation of the region field (to make it clear that if it is empty, Also, I have now specified the region in the tests to make the tests pass. Everything should be fine now. |
@therepanic No problem at all, thank you for the quick follow up! |
…-projects#3223) Signed-off-by: Andrey Litvitski <[email protected]>
I forgot to use |
@ashakirin what do you think? |
Mind, that removing the default region is a breaking change. It has to come at 1.1.x |
@markpollack, @tzolov: basically this change makes sense in combination with #823 fix, but it can break existing code based on fixed default region (Bedrock "us-east-1" has the most comprehensive model selection). Plan for next minor release? |
If our region is empty, then
DefaultAwsRegionProviderChain
will be used, which will search for the region in every possible way. This should be better for us, instead of setting the default field ourselves.Resolves: #3223