Skip to content
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

Remove redundant project_id validation in VertexAIModel to support cross-project service accounts #802

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

muffu-7
Copy link

@muffu-7 muffu-7 commented Jan 29, 2025

Changes
This PR removes an unnecessary validation check in the VertexAIModel class that compared the provided project_id with the credentials' project ID. Previously, the code enforced that these values must match, which is not required in Google Cloud Platform (GCP) workflows.

Rationale
In GCP, it is valid and common to use a service account from one project to interact with resources in another project. For example:

  • Centralized service account management in a "shared" project.

  • Cross-project API access (e.g., Vertex AI models in a different project).

The removed validation check prevented this legitimate use case by raising an error when the provided project_id differed from the credentials' project ID. This change prioritizes the explicitly provided project_id parameter, aligning with GCP's flexibility.

Impact

  • Users can now specify a project_id that differs from the service account's associated project.

  • Enables cross-project resource access patterns without validation errors.

  • Maintains backward compatibility — if no project_id is provided, credentials' project ID is still used as before.

…d_project_id and self.project_id can be different and self.project_id should be given priority over cred_project_id.
@muffu-7
Copy link
Author

muffu-7 commented Jan 29, 2025

Please have a look at this PR. It fails the tests because I have proposed a breaking change that doesn't align with your test cases. The tests expect a UserError exception to be raised, which is exactly what I am trying to prevent in this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant