-
Notifications
You must be signed in to change notification settings - Fork 67
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(weave): Implement VertexAI integration #2743
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
||
trace_name = op_name_from_ref(call.op_name) | ||
assert trace_name == "vertexai.GenerativeModel.generate_content" | ||
assert call.output is not 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.
can you make a more relevant assert using a mock? This is just checking if anything populated at all.
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.
I added cassettes for test_content_generation
and test_content_generation_stream
. However, I'm unable to generate them for test_content_generation_async
and test_content_generation_async_stream
.
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.
Do cassettes work here? I thought this used the google grpc stuff under the hood?
In the case where they don't work, you (or gpt!) will need to mock out the request/response manually
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.
@andrewtruong Do you mean that we need to write the cassettes manually for those specific functions? Or is there another way?
It's unfortunate we can't use VCRpy here. Instead of mocking, you can put a 5x retry on the test. It's not great, but given how fast these things move it might be the best option |
Your pytest markers should be called |
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.
Approving with the understanding that you have thoroughly checked the integration since these tests are skipped
Description
This PR adds the autopatch integration with VertexAI Generative Models API.
Supported cases
Sync generation
Without Streaming
Sample trace
With Streaming
Sample trace
Async Generation
Without Streaming
Sample trace
With Streaming
Sample trace