fix: Allow passing boto3 config to all AWS Bedrock classes

deepset-ai / haystack-core-integrations

Additional packages (components, document stores and the likes) to extend the capabilities of Haystack version 2.0 and onwards

https://haystack.deepset.ai

Apache License 2.0

121 stars 119 forks source link

fix: Allow passing boto3 config to all AWS Bedrock classes #1166

Open AnesBenmerzoug opened 2 weeks ago

AnesBenmerzoug commented 2 weeks ago

Related PR

follow-up to #1135

Proposed Changes:

Updated class init signatures to allow passing additional and optional boto3_config dictionary and use it when initializing boto3 client.
Updated tests accordingly

How did you test it?

Tested locally by running hatch test

Notes for the reviewer

This is a follow up to PR !1135 that simply applies the same change to all other aws bedrock classes (AmazonBedrockDocumentEmbedder, AmazonBedrockTextEmbedder, AmazonBedrockChatGenerator).

Checklist

[X] I have read the contributors guidelines and the code of conduct
~I have updated the related issue with new insights and changes~
[X] I added unit tests and updated the docstrings
[X] I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test:.

CLAassistant commented 2 weeks ago

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: AnesBenmerzoug
:white_check_mark: davidsbatista
:x: HaystackBot
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

AnesBenmerzoug commented 1 week ago

@davidsbatista I could not completely sign the CLA because I accidentaly some of the commits without configuring my email address. Should I leave it like this or is it okay to update the author of those commits and force-push the change?

davidsbatista commented 1 week ago

@davidsbatista I could not completely sign the CLA because I accidentaly some of the commits without configuring my email address. Should I leave it like this or is it okay to update the author of those commits and force-push the change?

You need to sign the CLA in order for the PR to be merged, so it seems you will need to update the author of these commits.

davidsbatista commented 1 week ago

@AnesBenmerzoug thanks for this contribution! I've left some comments regarding backward compatibility. I've also asked my colleague @vblagoje to do a second review.

davidsbatista commented 1 week ago

@AnesBenmerzoug remove the parameterization of all the tests, and instead do a test such that:

Create and serialize a new instance without boto3config, then load/deserialize that object, and assert that new deserialized object is exactly the same as the initially created instance