-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add vertex AI #85
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 vertex AI #85
Conversation
Deploying pydantic-ai with
|
Latest commit: |
73ad697
|
Status: | ✅ Deploy successful! |
Preview URL: | https://25c1191d.pydantic-ai.pages.dev |
Branch Preview URL: | https://vertexai.pydantic-ai.pages.dev |
bf4a807
to
8b3b7d1
Compare
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'm not familiar with the details of the codebase but in general looks good to me!
This is async in case slow/async config checks need to be performed that can't be done in `__init__`. | ||
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'm not sure this needs to be clarified in the docstring, I think it's okay if that's just how it is.
agent_model = m.agent_model({}, True, None) | ||
agent_model = await m.agent_model({}, True, 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.
I will say that it might be nice if these are forced to be keyword arguments, it'd be easy to mess up and do {}, None, True
or something and with None being falsely and whatnot it might fail in surprising ways.
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 is basically an internal API, otherwise I would agree.
patch = mocker.patch( | ||
'pydantic_ai.models.vertexai.google.auth.default', | ||
return_value=(NoOpCredentials(), 'my-project-id'), | ||
) |
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 tend to prefer passing this stuff in as parameters, e.g. have a parameter called _auth_provider: AuthProviderProtocol
or something and you pass in a test fake, maybe a mock. You wouldn't be able to do this monkey patching business in Rust 😉
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'd rather keen the code clear and mock like this.
Example Usage
With the default google project already configured in your environment:
Or using a service account JSON file: