[Feat] Integrate Bedrock & Vertex AI Embedding Adapters #156
Conversation
pk-zipstack
requested review from
chandrasekharan-zipstack,
gaya3-zipstack and
a team
There was a problem hiding this comment.
@gaya3-zipstack do you see any benefit of adding this file and similarly for other adapters as well? If not, let's remove it since we anyway treat everything as subpackages
There was a problem hiding this comment.
I don't see us leveraging this. We can remove it. But better we be consistent and do it all across..
There was a problem hiding this comment.
@gaya3-zipstack @chandrasekharan-zipstack, Is removing all the toml files of adapters in this PR feasible?
| return "/icons/adapter-icons/Bedrock.png" | ||
|
|
||
| @staticmethod | ||
| def get_json_schema() -> str: |
There was a problem hiding this comment.
NIT: Check if you define this in the base class alone and avoid redefining in each implementation
| except Exception as e: | ||
| raise AdapterError(str(e)) | ||
|
|
||
| def test_connection(self) -> bool: |
There was a problem hiding this comment.
NIT: I see that the same is used in all implementations. You can move this to the base class and not define it in each implementation
| embedding: BaseEmbedding = VertexTextEmbedding( | ||
| model_name=self.config.get(Constants.MODEL), | ||
| project=self.config.get(Constants.PROJECT), | ||
| credentials=credentials, |
There was a problem hiding this comment.
@pk-zipstack Can you please check if there are alternate ways to pass in credentials other than service account json? I remeber reading that we can also pass in keys. We can discuss and see if we need to add that option also. If at all we are supporting anything of that sort, we may have to do it for Vertex LLM also.
There was a problem hiding this comment.
@gaya3-zipstack, I remember reading about it too... I'll re-check once and we can discuss about it.
There was a problem hiding this comment.
Due to the task's urgency, I wanted to follow what the LLM followed to ship it quickly.
Co-authored-by: Chandrasekharan M <[email protected]> Signed-off-by: Praveen Kumar <[email protected]>
Signed-off-by: Praveen Kumar <[email protected]>
What
Added adapter support for
BedrockandVertext AIEmbedding modelsWhy
...
How
...
Relevant Docs
Related Issues or PRs
Dependencies Versions / Env Variables
Notes on Testing
Screenshots
...
Checklist
I have read and understood the Contribution Guidelines.