-
Notifications
You must be signed in to change notification settings - Fork 380
fix: suppress no project id warning #160
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: suppress no project id warning #160
Conversation
@@ -65,6 +65,8 @@ def init( | |||
""" | |||
if project: | |||
self._project = project | |||
if not os.environ.get(google.auth.environment_vars.PROJECT): | |||
os.environ[google.auth.environment_vars.PROJECT] = self._project |
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 seems like it would cause unintended side effects as other SDKs may reference this environment variable. I think we need to:
- Determine the source of the warning and the call site in the SDK that leads to warning being logged.
- See if we can avoid calling that method when we have project or pass project to the method.
It looks like this is the source of the warning:
https://github.com/googleapis/google-auth-library-python/blob/master/google/auth/_default.py#L251
My first guess is that it's called here:
_, project_id = google.auth.default() |
But I suspect that assumption is incorrect because, if the project set, it shouldn't invoke that method. So the issue may lie elsewhere.
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.
Yep, GOOGLE_CLOUD_PROJECT
is one of the standardized environment variables respected across libraries so overriding this may have unintended side effects. docs
FWIW the existing behavior makes sense to me - the warning only seems to be raised if the user has both (1) not passed a project
to __init__
and (2) not set a project in the environment.
python-aiplatform/google/cloud/aiplatform/initializer.py
Lines 44 to 67 in 4459fff
def init( | |
self, | |
*, | |
project: Optional[str] = None, | |
location: Optional[str] = None, | |
experiment: Optional[str] = None, | |
staging_bucket: Optional[str] = None, | |
credentials: Optional[auth_credentials.Credentials] = None, | |
): | |
"""Updates common initalization parameters with provided options. | |
Args: | |
project (str): The default project to use when making API calls. | |
location (str): The default location to use when making API calls. If not | |
set defaults to us-central-1 | |
experiment (str): The experiment to assign | |
staging_bucket (str): The default staging bucket to use to stage artifacts | |
when making API calls. In the form gs://... | |
credentials (google.auth.crendentials.Crendentials): The default custom | |
credentials to use when making API calls. If not provided crendentials | |
will be ascertained from the environment. | |
""" | |
if project: | |
self._project = project |
python-aiplatform/google/cloud/aiplatform/initializer.py
Lines 79 to 100 in 4459fff
@property | |
def project(self) -> str: | |
"""Default project.""" | |
if self._project: | |
return self._project | |
project_not_found_exception_str = ( | |
"Unable to find your project. Please provide a project ID by:" | |
"\n- Passing a constructor argument" | |
"\n- Using aiplatform.init()" | |
"\n- Setting a GCP environment variable" | |
) | |
try: | |
_, project_id = google.auth.default() | |
except GoogleAuthError: | |
raise GoogleAuthError(project_not_found_exception_str) | |
if not project_id: | |
raise ValueError(project_not_found_exception_str) | |
return 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 thought at the beginning the warning was from
_, project_id = google.auth.default() |
But using Dataset as example, the warning comes from
python-aiplatform/google/cloud/aiplatform_v1beta1/services/dataset_service/transports/grpc.py
Line 130 in be78cde
credentials, _ = auth.default( |
I think this warning has nothing to do with MBSDK, it is more for a warning from google.auth, to indicate the gcloud nor environment variable is not set. And the google.auth.default() is triggered when there is a transport grpc call.
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.
@sasha-gitg , @busunkim96
modified approached: when no credentials is provided during MBSDK initialization, set the credentials initializer.global_config._credentials to google.auth.default's credentials, thus during any service client instantiation, google.auth.default will not be called again.
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.
LGTM! Thanks for putting this together. One minor change for consistency.
@@ -75,6 +75,12 @@ def init( | |||
self._staging_bucket = staging_bucket | |||
if credentials: | |||
self._credentials = credentials | |||
else: | |||
logger = logging.getLogger("google.auth._default") |
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.
Please move this logic into the credentials
property to be consistent with the default project retrieval. Default project
is pulled at time of accessing the property while, in this case, default credential
is pulled when initializing the config.
cd26331
to
c7ea670
Compare
* fix: unblock builds (#132) * chore: Update README with Experimental verbiage (#131) * fix: Fixed comments (#116) Co-authored-by: Ivan Cheung <[email protected]> * feat: Implements a wrapped client that instantiates the client at every API invocation (#139) * feat: Added optional model args for custom training (#129) * Added optional model args * fix: Removed etag * fix: Added predict schemata and fixed type error * fix: Added description and fixed predict_schemata * Added _model_serving_container_command, _model_serving_container_args, env=self._model_serving_container_environment_variables and _model_serving_container_ports * fix: Ran linter * fix: Added tests for model_instance_schema_uri, model_parameters_schema_uri and model_prediction_schema_uri * fix: Fixed env and ports and added tests * fix: Removed model_labels * fix: Moved container spec creation into init function * fix: Fixed docstrings * fix: Moved import to be alphabetical * fix: Moved model creation to init function * fix: Fixed predict_schemata * fix: simplified predict schemata * fix: added linter * fix: Fixed trailing comma * fix: Removed CustomTrainingJob private fields * fix: Fixed model tests * fix: Set managed_model to None Co-authored-by: Ivan Cheung <[email protected]> * Fix: refactor class constructor for retrieving resource (#125) * Added property and abstract method _getter_method and _resource_noun, implemented method _get_gca_resource to class AiPlatformResourceNoun; Added _resource_noun, _getter_method, to Dataset, Model, Endpoint, subclasses of _Job, _TrainingJob, refactored (_)get_* and utils.full_resource_name in class constructor to self._get_gca_resource to Dataset, Model, Endpoint, _Job * Added return value in _get_gca_resource, added method _sync_gca_resource in AiPlatformResourceNoun class; removed job_type, updated status method with _sync_gca_resource in _Job class * fix: added return type and lint issues * fix: merge conflict issue with models.py * fix: F401 'abc' imported but unused * chore: merge main into dev (#154) * test: Dataset integration tests (#126) * Add dataset.metadata.text to schemas * Add first integation tests, Dataset class * Make teardown work if test fails, update asserts * Change test folder name, enable system tests * Hide test_base, test_end_to_end for Kokoro CI bug * Add GCP Project env var to Kokoro presubmit cfg * Restore presubmit cfg, drop --quiet in unit tests * Restore test_base, test_end_to_end to find timeout * Skip tests depending on persistent resources * Use auth default creds for system tests * Drop unused import os * feat: specialized dataset classes, fix: datasets refactor (#153) * feat: Refactored Dataset by removing intermediate layers * Added image_dataset and tabular_dataset subclass * Moved metadata_schema_uri responsibility to subclass to enable forecasting * Moved validation logic for tabular into Dataset._create_tabular * Added validation in image_dataset and fixed bounding_box schema error * Removed import_config * Fixed metadata_schema_uri * Fixed import and subclasses * Added EmptyNontabularDatasource * change import_metadata to ioformat * added datasources.py * added support of multiple gcs_sources * fix: default (empty) dataset_metadata need to be set to {}, not None * 1) imported datasources 2) added _support_metadata_schema_uris and _support_import_schema_classes 3) added getter and setter/validation for resource_metadata_schema_uri, metadata_schema_uri, and import_schema_uri 4) fixed request_metadata, data_item_labels 5) encapsulated dataset_metadata, and import_data_configs 6) added datasource configuration logic * added image_dataset.py and tabular_dataset.py * fix: refactor - create datasets modeule * fix: cleanup __init__.py * fix: data_item_labels * fix: docstring * fix: - changed NonTabularDatasource.dataset_metadata default to None - updated NonTabularDatasource docstring - changed gcs_source type hint with Union - changed _create_and_import to _create_encapsulated with datasource - removed subclass.__init__ and irrelevant parameters in create * fix: import the module instead of the classes for datasources * fix: removed all validation for import_schema_uri * fix: set parameter default to immutable * fix: replaced Datasource / DatasourceImportable abstract class instead of a concrete type * fix: added examples for gcs_source * fix: - remove Sequence from utils.py - refactor datasources.py to _datasources.py - change docstring format to arg_name (arg_type): convention - change and include the type signature _supported_metadata_schema_uris - change _validate_metadata_schema_uri - refactor _create_encapsulated to _create_and_import - refactor to module level imports - add tests for ImageDataset and TabularDataset * fix: remove all labels * fix: remove Optional in docstring, add example for bq_source * test: add import_data raise for tabular dataset test * fix: refactor datasource creation with create_datasource * fix: lint Co-authored-by: Ivan Cheung <[email protected]> * feat: Add AutoML Image Training Job class (#152) * Add AutoMLImageTrainingJob, tests, constants * Address reviewer comments * feat: Add custom container support (#164) * chore: merge main into dev (#162) * fix: suppress no project id warning (#160) * fix: suppress no project id warning * fix: temporary suppress logging.WARNING and set credentials as google.auth.default credentials * fix: move default credentials config to credentials property * fix: add property setter for credentials to avoid everytime reset * fix: Fixed wrong key value for multilabel (#168) Co-authored-by: Ivan Cheung <[email protected]> * feat: Add delete methods, add list_models and undeploy_all for Endpoint class (#165) * Endpoint list_models, delete, undeploy_all WIP * Finish delete + undeploy methods, tests * Add global pool teardowns for test timeout issue * Address reviewer comments, add async support * fix: Fixed bug causing training failure for object detection (#171) Co-authored-by: Ivan Cheung <[email protected]> * fix: Support intermediary BQ Table for Custom Training (#166) * chore: add AutoMLImageTrainingJob to aiplatform namespace (#173) * fix: Unblock build (#174) * fix: default credentials config related test failures (#167) * fix: suppress no project id warning * fix: temporary suppress logging.WARNING and set credentials as google.auth.default credentials * fix: move default credentials config to credentials property * fix: add property setter for credentials to avoid everytime reset * fix: tests for set credentials to default when default not provided * fix: change credentials with initializer default when not provided in AiPlatformResourceNoun * fix: use credential mock in tests * fix: lint Co-authored-by: sasha-gitg <[email protected]> Co-authored-by: Ivan Cheung <[email protected]> Co-authored-by: Ivan Cheung <[email protected]> Co-authored-by: Morgan Du <[email protected]> Co-authored-by: Vinny Senthil <[email protected]>
* fix: unblock builds (#132) * chore: Update README with Experimental verbiage (#131) * fix: Fixed comments (#116) Co-authored-by: Ivan Cheung <[email protected]> * feat: Implements a wrapped client that instantiates the client at every API invocation (#139) * feat: Added optional model args for custom training (#129) * Added optional model args * fix: Removed etag * fix: Added predict schemata and fixed type error * fix: Added description and fixed predict_schemata * Added _model_serving_container_command, _model_serving_container_args, env=self._model_serving_container_environment_variables and _model_serving_container_ports * fix: Ran linter * fix: Added tests for model_instance_schema_uri, model_parameters_schema_uri and model_prediction_schema_uri * fix: Fixed env and ports and added tests * fix: Removed model_labels * fix: Moved container spec creation into init function * fix: Fixed docstrings * fix: Moved import to be alphabetical * fix: Moved model creation to init function * fix: Fixed predict_schemata * fix: simplified predict schemata * fix: added linter * fix: Fixed trailing comma * fix: Removed CustomTrainingJob private fields * fix: Fixed model tests * fix: Set managed_model to None Co-authored-by: Ivan Cheung <[email protected]> * Fix: refactor class constructor for retrieving resource (#125) * Added property and abstract method _getter_method and _resource_noun, implemented method _get_gca_resource to class AiPlatformResourceNoun; Added _resource_noun, _getter_method, to Dataset, Model, Endpoint, subclasses of _Job, _TrainingJob, refactored (_)get_* and utils.full_resource_name in class constructor to self._get_gca_resource to Dataset, Model, Endpoint, _Job * Added return value in _get_gca_resource, added method _sync_gca_resource in AiPlatformResourceNoun class; removed job_type, updated status method with _sync_gca_resource in _Job class * fix: added return type and lint issues * fix: merge conflict issue with models.py * fix: F401 'abc' imported but unused * chore: merge main into dev (#154) * test: Dataset integration tests (#126) * Add dataset.metadata.text to schemas * Add first integation tests, Dataset class * Make teardown work if test fails, update asserts * Change test folder name, enable system tests * Hide test_base, test_end_to_end for Kokoro CI bug * Add GCP Project env var to Kokoro presubmit cfg * Restore presubmit cfg, drop --quiet in unit tests * Restore test_base, test_end_to_end to find timeout * Skip tests depending on persistent resources * Use auth default creds for system tests * Drop unused import os * feat: specialized dataset classes, fix: datasets refactor (#153) * feat: Refactored Dataset by removing intermediate layers * Added image_dataset and tabular_dataset subclass * Moved metadata_schema_uri responsibility to subclass to enable forecasting * Moved validation logic for tabular into Dataset._create_tabular * Added validation in image_dataset and fixed bounding_box schema error * Removed import_config * Fixed metadata_schema_uri * Fixed import and subclasses * Added EmptyNontabularDatasource * change import_metadata to ioformat * added datasources.py * added support of multiple gcs_sources * fix: default (empty) dataset_metadata need to be set to {}, not None * 1) imported datasources 2) added _support_metadata_schema_uris and _support_import_schema_classes 3) added getter and setter/validation for resource_metadata_schema_uri, metadata_schema_uri, and import_schema_uri 4) fixed request_metadata, data_item_labels 5) encapsulated dataset_metadata, and import_data_configs 6) added datasource configuration logic * added image_dataset.py and tabular_dataset.py * fix: refactor - create datasets modeule * fix: cleanup __init__.py * fix: data_item_labels * fix: docstring * fix: - changed NonTabularDatasource.dataset_metadata default to None - updated NonTabularDatasource docstring - changed gcs_source type hint with Union - changed _create_and_import to _create_encapsulated with datasource - removed subclass.__init__ and irrelevant parameters in create * fix: import the module instead of the classes for datasources * fix: removed all validation for import_schema_uri * fix: set parameter default to immutable * fix: replaced Datasource / DatasourceImportable abstract class instead of a concrete type * fix: added examples for gcs_source * fix: - remove Sequence from utils.py - refactor datasources.py to _datasources.py - change docstring format to arg_name (arg_type): convention - change and include the type signature _supported_metadata_schema_uris - change _validate_metadata_schema_uri - refactor _create_encapsulated to _create_and_import - refactor to module level imports - add tests for ImageDataset and TabularDataset * fix: remove all labels * fix: remove Optional in docstring, add example for bq_source * test: add import_data raise for tabular dataset test * fix: refactor datasource creation with create_datasource * fix: lint Co-authored-by: Ivan Cheung <[email protected]> * feat: Add AutoML Image Training Job class (#152) * Add AutoMLImageTrainingJob, tests, constants * Address reviewer comments * feat: Add custom container support (#164) * chore: merge main into dev (#162) * fix: suppress no project id warning (#160) * fix: suppress no project id warning * fix: temporary suppress logging.WARNING and set credentials as google.auth.default credentials * fix: move default credentials config to credentials property * fix: add property setter for credentials to avoid everytime reset * fix: Fixed wrong key value for multilabel (#168) Co-authored-by: Ivan Cheung <[email protected]> * feat: Add delete methods, add list_models and undeploy_all for Endpoint class (#165) * Endpoint list_models, delete, undeploy_all WIP * Finish delete + undeploy methods, tests * Add global pool teardowns for test timeout issue * Address reviewer comments, add async support * fix: Fixed bug causing training failure for object detection (#171) Co-authored-by: Ivan Cheung <[email protected]> * fix: Support intermediary BQ Table for Custom Training (#166) * chore: add AutoMLImageTrainingJob to aiplatform namespace (#173) * fix: Unblock build (#174) * fix: default credentials config related test failures (#167) * fix: suppress no project id warning * fix: temporary suppress logging.WARNING and set credentials as google.auth.default credentials * fix: move default credentials config to credentials property * fix: add property setter for credentials to avoid everytime reset * fix: tests for set credentials to default when default not provided * fix: change credentials with initializer default when not provided in AiPlatformResourceNoun * fix: use credential mock in tests * fix: lint Co-authored-by: sasha-gitg <[email protected]> * Fix: pass bq_destination to input data config when using training script (#181) * fix: pass bigquery destination * fix: add tests and formatting Co-authored-by: Ivan Cheung <[email protected]> Co-authored-by: Ivan Cheung <[email protected]> Co-authored-by: Morgan Du <[email protected]> Co-authored-by: Vinny Senthil <[email protected]>
Fixes https://b.corp.google.com/issues/176166878