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

fix: make "project-id" parameter optional during initialization #1141

Merged
merged 7 commits into from
Oct 17, 2024

Conversation

Amnah199
Copy link
Contributor

@Amnah199 Amnah199 commented Oct 16, 2024

Related Issues

Proposed Changes:

The project is automatically set during authentication using Google Cloud ADCs, which is required for configuration. Therefore, we can make the project-id optional in the component init.

How did you test it?

  • Ran existing tests
  • Manual testing using examples

Notes for the reviewer

Checklist

@Amnah199 Amnah199 requested a review from a team as a code owner October 16, 2024 14:05
@Amnah199 Amnah199 requested review from julian-risch and removed request for a team October 16, 2024 14:05
@github-actions github-actions bot added integration:google-vertex type:documentation Improvements or additions to documentation labels Oct 16, 2024
Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite good to me! I suggest that we remove the project_id param from the initialization of the VertexAIGeminiChatGenerator too. Other than that ready to be merged.

I checked what error message we get if no project id is provided at all.

    raise GoogleAuthError(project_not_found_exception_str) from exc
google.auth.exceptions.GoogleAuthError: Unable to find your project. Please provide a project ID by:
- Passing a constructor argument
- Using vertexai.init()
- Setting project using 'gcloud config set project my-project'
- Setting a GCP environment variable
- To create a Google Cloud project, please follow guidance at https://developers.google.com/workspace/guides/create-project

Looks good.

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more change request: let's remove project_id from the test_to_dict and test_from_dict. We can keep it in test_to_dict_with_params and test_from_dict_with_param.

@Amnah199
Copy link
Contributor Author

VertexAIGeminiChatGenerator is already updated. I'll update the tests too 👍

@Amnah199 Amnah199 requested a review from julian-risch October 17, 2024 12:52
@Amnah199
Copy link
Contributor Author

@julian-risch updated the examples and also removed project_id from all tests except init. Let me know WDYT.

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@Amnah199 Amnah199 merged commit 2f12690 into main Oct 17, 2024
16 checks passed
@Amnah199 Amnah199 deleted the remove-project-id-vertexai branch October 17, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:google-vertex type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: google-vertex-ai forces you to specify the project_id as a parameter
2 participants