-
Notifications
You must be signed in to change notification settings - Fork 109
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
feat: enable streaming for VertexAIGeminiChatGenerator #1014
Conversation
@julian-risch I've added you for review since this is similar to the PR #1012. |
...s/google_vertex/src/haystack_integrations/components/generators/google_vertex/chat/gemini.py
Show resolved
Hide resolved
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 very good to me already! We should just add type annotations in two functions. Otherwise it'S ready to be merged. And I am looking forward to tests as tracked by #1019
...s/google_vertex/src/haystack_integrations/components/generators/google_vertex/chat/gemini.py
Outdated
Show resolved
Hide resolved
...s/google_vertex/src/haystack_integrations/components/generators/google_vertex/chat/gemini.py
Outdated
Show resolved
Hide resolved
responses.append(streaming_chunk.content) | ||
|
||
combined_response = "".join(responses).lstrip() | ||
return [ChatMessage.from_system(content=combined_response)] |
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.
This will be changed to assistant
role in another PR. I am tracking it here.
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.
LGTM! 👍
* Add streaming_callback in init() and run()
Related Issues
Proposed Changes:
Add
streaming_callback
param toVertexAIGeminiChatGenerator
to enable streaming.How did you test it?
Manually testing as this component has no written tests.
Notes for the reviewer
This feature can also be implemented without using
StreamingChunk
but I decided to use it for consistency with other implementations we have in Bedrock, OpenAI etc.Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.