Use AWS_DEFAULT_REGION for bedrock if provided #157
Conversation
src/autogluon/assistant/llm/llm.py
Outdated
| try: | ||
| # TODO: Remove hardcoding AWS region | ||
| bedrock = boto3.client("bedrock", region_name="us-west-2") | ||
| bedrock = boto3.client("bedrock", region_name=os.environ.get("AWS_DEFAULT_REGION", "us-west-2")) |
There was a problem hiding this comment.
If you leave out region_name in the client creation, boto3 handles this fallback behavior automatically. This might be cleaner where we don't need to take care of passing the region everytime a boto3 client initiated in codes
There was a problem hiding this comment.
I agree we could simply remove the region_name kwarg, the only thing being added here is that we have a fallback in case the user doesn't set AWS_DEFAULT_REGION. But I'm happy to remove this as well.
There was a problem hiding this comment.
The default behavior uses AWS_DEFAULT_REGION or the AWS config file, and as a last resort, it resorts to IMDS for EC2. If none of these sources provide a region resolution, a NoRegionError is raised to ensure that the user has to explicitly set the region in the running environment. I believe this approach is better than selecting a region on behalf of the user.
| eval_model: str = "lightgbm", | ||
| # TODO: Remove hardcoding AWS region | ||
| region_name: str = "us-west-2", | ||
| region_name: str = os.environ.get("AWS_DEFAULT_REGION", "us-west-2"), |
There was a problem hiding this comment.
@tonyhoo it is still required to pass the region_name here as for CAAFE, so I think it is okay to keep this.
Later after the release I'm going to refactor CAAFE, and then this argument can also be removed.
There was a problem hiding this comment.
I think it would be better to keep the region setting consistent across the repo
There was a problem hiding this comment.
It's not possible to remove it from CAAFE until we make changes in CAAFE upstream. Would you suggest that I also make changes upstream? See details here:
|
Solved in #204 |
3 participants
Description
How Has This Been Tested?
pytest tests/)aga run knot_theory --config_overrides "time_limit=120, feature_transformers.enabled_models=[CAAFE]"Configuration Changes
Type of Change