chore(bigframes): Add support for bigframes-only project id env var#16859
chore(bigframes): Add support for bigframes-only project id env var#16859TrevorBergeron wants to merge 1 commit intomainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes the logic for determining the default Google Cloud project ID by introducing a new env configuration module and updating the ClientsProvider to require a project ID. The review feedback highlights critical NameError issues in the new get_default_project_id function and the _get_bqclient_and_project helper, where variables were incorrectly referenced. Additionally, there are suggestions to remove an unused import and add a missing newline at the end of the new configuration file to align with PEP 8 standards.
| # 2. Project set in the environment | ||
| # 3. Project associated with the default credentials | ||
| maybe_from_env = ( | ||
| config.options.bigquery.project |
There was a problem hiding this comment.
The name config is not defined in this scope, which will cause a NameError. Based on the import from bigframes._config import options, you should use options to access the configuration.
| config.options.bigquery.project | |
| options.bigquery.project |
| requests_transport_adapters=config.options.bigquery.requests_transport_adapters, | ||
| ) | ||
| return clients_provider.bqclient, clients_provider._project | ||
| return clients_provider.bqclient, project |
There was a problem hiding this comment.
The variable project is not defined in this scope, which will cause a NameError. You should use project_id, which is defined on line 660.
| return clients_provider.bqclient, project | |
| return clients_provider.bqclient, project_id |
|
|
||
| import os | ||
|
|
||
| from google.auth import default as auth_default |
There was a problem hiding this comment.
This import of auth_default is unused and can be removed.
References
- PEP 8 suggests removing unused imports to keep the code clean and avoid unnecessary dependencies. (link)
| "Try setting `bigframes.options.bigquery.project` first." | ||
| ) | ||
|
|
||
| return creds_project No newline at end of file |
There was a problem hiding this comment.
It is a standard convention to end files with a newline character. Please add a newline at the end of the file.
| return creds_project | |
| return creds_project | |
References
- PEP 8 recommends that files should end with a single newline character. (link)
1 participant
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕