Skip to content

Custom user agent#277

Merged
brnaba-aws merged 8 commits intomainfrom
custom-user-agent
Mar 26, 2025
Merged

Custom user agent#277
brnaba-aws merged 8 commits intomainfrom
custom-user-agent

Conversation

@brnaba-aws
Copy link
Copy Markdown
Collaborator

Added custom user agent

Copy link
Copy Markdown
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hey @brnaba-aws! Overall this PR looks good, I just left some small comments that make sense you take a look.

Comment on lines +31 to +33
self.bedrock_agent_client = boto3.client('bedrock-agent-runtime')

user_agent.register_feature_to_client(self.bedrock_agent_client, feature="bedrock-flows-agent")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if removing region was intentional or not.

)
else:
self.client = boto3.client('bedrock-runtime')
region_name=options.region or os.environ.get('AWS_REGION'))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now you are forcing the condition of having options.region or the AWS_REGION env. It is certainly my fault for not understanding the entire codebase, but if that is not the explicit intention, you are probably creating a regression.

Comment on lines +44 to +45
chat_history: list[ConversationMessage],
additional_params: Optional[dict[str, str]] = None) -> ConversationMessage:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure which Python versions you support in the project, but if you support Python < 3.8 you will have a problem here. list and dict were introduced in the PEP585 and only supports Python 3.9+. Or you revert this or add future statement on top of this file.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

=3.11

@brnaba-aws brnaba-aws merged commit da3eb46 into main Mar 26, 2025
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