Skip to content
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

Add AnthropicVertexChatGenerator component #1192

Merged
merged 8 commits into from
Nov 15, 2024

Conversation

Amnah199
Copy link
Contributor

@Amnah199 Amnah199 commented Nov 14, 2024

Related Issues

Proposed Changes:

  • Create AnthropicVertexChatGenerator class that inherits from AnthropicChatGenerator
  • Replace api_key with parameters needed for AnthropicVertex

How did you test it?

  • Added new tests for the component
  • Locally tested some examples

Notes for the reviewer

A documentation will be created for this component

Checklist

@github-actions github-actions bot added the type:documentation Improvements or additions to documentation label Nov 14, 2024
@Amnah199 Amnah199 marked this pull request as ready for review November 14, 2024 21:03
@Amnah199 Amnah199 requested a review from a team as a code owner November 14, 2024 21:03
@Amnah199 Amnah199 requested review from silvanocerza and removed request for a team November 14, 2024 21:03
@Amnah199 Amnah199 changed the title Create adapter class and add AnthropicVertex Add AnthropicVertexChatGenerator component Nov 14, 2024
Comment on lines +96 to +97
self.region = region or os.environ.get("REGION")
self.project_id = project_id or os.environ.get("PROJECT_ID")
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be Secrets I think. If you do it like this you can leak them when serializing the component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case project_id and region do not grant access to GCP resources on their own, as proper authentication and permissions are required. So I thought these values are not sensitive data; but more of configuration parameters. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, we treated them as secrets on some other components though. Good for me to change this.

In any case I would make them mandatory arguments. If the user wants to pass them as env vars that's up to them.

Copy link
Contributor

@silvanocerza silvanocerza left a comment

Choose a reason for hiding this comment

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

Great job! 👏

@silvanocerza silvanocerza merged commit e21ce0c into main Nov 15, 2024
10 checks passed
@silvanocerza silvanocerza deleted the add-vertexai-to-anthropic branch November 15, 2024 13:03
Amnah199 added a commit to ttmenezes/haystack-core-integrations that referenced this pull request Nov 15, 2024
* Created a model adapter

* Create adapter class and add VertexAPI

* Add chat generator for Anthropic Vertex

* Add tests

* Small fix

* Improve doc_strings

* Make project_id and region mandatory params

* Small fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Anthropic integration to AnthropicVertex
2 participants