-
Notifications
You must be signed in to change notification settings - Fork 199
feat: Add Together AI integration #2268
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
Conversation
Generator and ChatGeneratorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks quite good to me already! I have a couple of change requests, most of them with suggestions of code changes. In addition, the repo README should list the integration https://github.com/deepset-ai/haystack-core-integrations/blob/main/README.md
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Show resolved
Hide resolved
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
integrations/together_ai/tests/test_together_ai_chat_generator.py
Outdated
Show resolved
Hide resolved
...rations/together_ai/src/haystack_integrations/components/generators/together_ai/generator.py
Outdated
Show resolved
Hide resolved
...rations/together_ai/src/haystack_integrations/components/generators/together_ai/generator.py
Outdated
Show resolved
Hide resolved
...rations/together_ai/src/haystack_integrations/components/generators/together_ai/generator.py
Outdated
Show resolved
Hide resolved
...gether_ai/src/haystack_integrations/components/generators/together_ai/chat/chat_generator.py
Show resolved
Hide resolved
|
Sorry for all the baseless typos in this PR! Will request review again once I have tested the component in deepset platform. |
| tools: Optional[Union[List[Tool], Toolset]] = None, | ||
| timeout: Optional[float] = None, | ||
| max_retries: Optional[int] = None, | ||
| http_client_kwargs: Optional[Dict[str, Any]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering whether we need this param http_client_kwargs here. Its compatible but not sure if it adds any value for this integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me to have it so that http client settings can be passed on to the OpenAIChatGenerator and I see verify=False in the example to disable ssl verification.
|
@julian-risch I tested this component as a custom component in a dc pipeline and it works. Would be nice to have another pass for the review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just some smaller changes to comments that I'll apply directly. The PR is ready to be merged. 👍
| tools: Optional[Union[List[Tool], Toolset]] = None, | ||
| timeout: Optional[float] = None, | ||
| max_retries: Optional[int] = None, | ||
| http_client_kwargs: Optional[Dict[str, Any]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me to have it so that http client settings can be passed on to the OpenAIChatGenerator and I see verify=False in the example to disable ssl verification.
Related Issues
Proposed Changes:
Add a new integration for TogetherAI that inherits from
OpenAIChatGeneratorHow did you test it?
Add new tests
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.