OLS-2588: Add google vertex provider for Gemini and Claude#2877
OLS-2588: Add google vertex provider for Gemini and Claude#2877raptorsun wants to merge 4 commits intoopenshift:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
fa4f807 to
a759596
Compare
148dff7 to
8e903b2
Compare
|
@raptorsun: This pull request references OLS-2588 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@raptorsun: This pull request references OLS-2588 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ols/app/models/config.py
Outdated
| and self.google_vertex_config == other.google_vertex_config | ||
| and self.tls_security_profile == other.tls_security_profile | ||
| ) | ||
| return False |
There was a problem hiding this comment.
Why do we need it? Pydantic v2's BaseModel already compares all fields by default. This is very error prone
There was a problem hiding this comment.
is it to say we should remove this eq function?
|
credentials_path is configured but never passed to the LLM constructors Compare with every other provider in the codebase: openai.py, watsonx.py, rhoai_vllm.py, rhelai_vllm.py, azure_openai.py — all do this:self.credentials = self.provider_config.credentials ... and then pass it to the LangChain constructorThe Google Vertex providers skip this entirely. Authentication works only because the operator separately sets the GOOGLE_APPLICATION_CREDENTIALS environment variable, and Google's client libraries pick it up via Application Default Credentials (ADC). The credentials_path in the config creates a false sense of control — an operator could point it at the wrong file and never notice because auth succeeds through the ambient env var. Both LangChain constructors accept an explicit credentials parameter: from google.oauth2 import service_account This needs to be fixed |
546dfd8 to
fb33e52
Compare
|
@blublinsky thank you very much for the review! |
| def default_params(self) -> dict[str, Any]: | ||
| """Construct and return structure with default LLM params.""" | ||
| self.project = self.provider_config.project_id | ||
| account_info = credentials_str_to_dict(self.provider_config.credentials) |
There was a problem hiding this comment.
credentials_str_to_dict(self.provider_config.credentials) is called unconditionally, but self.provider_config.credentials is None when credentials_path is not configured (read_secret returns None for missing paths). This crashes with a raw TypeError from json.loads(None) instead of a clear configuration error.
Other providers guard against this -- e.g. watsonx.py:56:
if self.credentials is None:
raise ValueError("Credentials must be specified")
Add a similar check before the credentials_str_to_dict call.
| """Construct and return structure with default LLM params.""" | ||
| self.project = self.provider_config.project_id | ||
| account_info = credentials_str_to_dict(self.provider_config.credentials) | ||
| self.credentials = service_account.Credentials.from_service_account_info( |
There was a problem hiding this comment.
credentials_str_to_dict(self.provider_config.credentials) is called unconditionally, but self.provider_config.credentials is None when credentials_path is not configured (read_secret returns None for missing paths). This crashes with a raw TypeError from json.loads(None) instead of a clear configuration error.
Other providers guard against this -- e.g. watsonx.py:56:
if self.credentials is None:
raise ValueError("Credentials must be specified")
Add a similar check before the credentials_str_to_dict call.
| wheel_packages="$wheel_packages,$EXTRA_WHEELS" | ||
| sed -i 's/"packages": "[^"]*"/"packages": "'"$wheel_packages"'"/' .tekton/lightspeed-service-pull-request.yaml | ||
| sed -i 's/"packages": "[^"]*"/"packages": "'"$wheel_packages"'"/' .tekton/lightspeed-service-push.yaml | ||
|
|
There was a problem hiding this comment.
he maturin==1.10.2 pin was removed, but maturin is only in EXTRA_WHEELS, not in $wheel_packages (derived from WHEEL_FILE). The new dedup loop on line 87-90 only strips BUILD_FILE entries matching names in $wheel_packages, so it doesn't cover maturin. If pybuild-deps emits a different maturin version than what the Red Hat registry provides, you'll get hash conflicts. Either restore the pin or extend the dedup loop to also cover EXTRA_WHEELS.
There was a problem hiding this comment.
Hermeto download build-requirements first, and then the rest of requirment files.
When a package like maturin is listed in both build requirements and other reqiurement files, it will be downloaded only once and the hash of that from build dependencies is often different than the package we have in the wheel requirment file.
So I remove the packages from build requirement file if the same package is also listed in the wheel requirement file.
Normally maturin is not a runtime dependency , its functionality is to manage rust environment, so a build time depenency.
I filter the wheel packages first from the build dependency file and then add extra wheels list for the hermeto config. So that we do not remove the package in EXTRA_WHEELS list that may cause some package never downloaded, if the package is not in the wheel requirement file.
fb33e52 to
6980913
Compare
1418021 to
12a1d6e
Compare
Adds a new `google_vertex` LLM provider type that enables using Anthropic Claude models through Google Cloud's Vertex AI. Uses `ChatAnthropicVertex` from `langchain-google-vertexai` which authenticates via Google ADC. Also bumps httpx to >=0.28.0 (required by langchain-google-vertexai) and updates provider.py to use the renamed `proxy` parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Haoyu Sun <hasun@redhat.com>
7d5d6bf to
7dec5ff
Compare
Signed-off-by: Haoyu Sun <hasun@redhat.com>
…_objects_api" on class object Signed-off-by: Haoyu Sun <hasun@redhat.com>
7dec5ff to
1ce47fa
Compare
|
@raptorsun: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
This PR adds 2 providers:
This PR replaces #2824
Here is an example of configuration to access Gemini and Claude models.
The credential file is the key file for service accounts in Google cloud.
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing