-
Notifications
You must be signed in to change notification settings - Fork 197
feat: add CometAPI integration with CometAPIChatGenerator class and configuration #2345
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
base: main
Are you sure you want to change the base?
Conversation
…Embedder and GoogleAIDocumentEmbedder
…entEmbedder and GoogleAITextEmbedder tests
…der to use private attributes for initialization
…document embedder and tests
…der and GoogleGenAITextEmbedder
Google genai embedders async runs
…IDocumentEmbedder
…atch_async method
…lank line in test file
…or CometAPIChatGenerator
Hi @sjrl, could you take a look at the PR and check if it includes everything you requested? |
integrations/cometapi/pyproject.toml
Outdated
# TODO: remove lint environment once this integration is properly typed | ||
# test environment should be used instead | ||
# https://github.com/deepset-ai/haystack-core-integrations/issues/1771 | ||
[tool.hatch.envs.lint] | ||
installer = "uv" | ||
detached = true | ||
dependencies = ["pip", "black>=23.1.0", "mypy>=1.0.0", "ruff>=0.0.243", "more-itertools"] |
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.
Could you follow the pyproject.toml
format used in the OpenRouter integration.
This TODO only exists in a few remaining integrations and we should avoid using one of those as an example.
@garybadwal could you also add a |
… and add new linting rules
@sjrl, I’ve added the files and made the changes. However, the tests are now failing—could it be due to the missing API key, or am I doing something wrong? Could you please check and let me know? |
@TensorNull, I’ve added these resources. Please let me know what all you’d like to include in the description, and I’ll update the README.md accordingly. |
…single file, update to synchronous methods, and enhance test coverage for initialization, running with parameters, and tool integration.
…CometAPIChatGenerator
Hi @sjrl and @TensorNull, I have added all the requested information and included the new test cases from the OpenRouter integration. All tests are now passing. Could you please review the PR and let me know if there’s anything you’d like me to change or add? |
Hi @sjrl, just checking if you or @TensorNull can give a review to the changes requested. So I can fix things if required. |
Related Issues
Proposed Changes
Added a new
CometAPIChatGenerator
class that extendsOpenAIChatGenerator
to support the Comet API as a backend for chat generation.api_base_url
to point tohttps://api.cometapi.com/v1
.COMET_API_KEY
environment variable (usingSecret.from_env_var
).OpenAIChatGenerator
.gpt-4o-mini
.This provides a seamless way to use CometAPI as a drop-in replacement for OpenAI within Haystack.
How did you test it?
CometAPIChatGenerator
with and without explicit parameters.OpenAIChatGenerator
.api_base_url
=https://api.cometapi.com/v1
).COMET_API_KEY
).Notes for the reviewer
CometAPIChatGenerator
class.OpenAIChatGenerator
logic, only customizingapi_base_url
and defaults.Checklist
feat:
in this case)