-
Notifications
You must be signed in to change notification settings - Fork 323
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
Feat: parse project location when passed full resource name to get apis #297
Feat: parse project location when passed full resource name to get apis #297
Conversation
a2cc4a8
to
3907e4e
Compare
google/cloud/aiplatform/base.py
Outdated
@@ -320,6 +320,16 @@ def _get_gca_resource(self, resource_name: str) -> proto.Message: | |||
project=self.project, | |||
location=self.location, | |||
) | |||
( |
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 think this check needs to happen earlier in the stack than _get_gca_resource
because the project/location of the object get set before this:
https://github.com/googleapis/python-aiplatform/blob/dev/google/cloud/aiplatform/datasets/dataset.py#L73
->
https://github.com/googleapis/python-aiplatform/blob/dev/google/cloud/aiplatform/base.py#L279
In this current implementation the location attribute and project attribute on the object may match the resource and client.
I think the most direct way to support this is to to add something like this to the AIPlatformResourceNoun base class:
def _get_and_validate_project_location(cls, project, location, resource_name_or_id):
if not project and not location: # no issue
return project, location
fields = extract_fields_from_resource_name(resource_name_or_id, cls._resource_noun)
if not fields: # not a resource name
return project, location
# skip validating project because it's not needed and would require we correlate project no. with project ids
if location and field.location != location: # this is a problem
raise runtimeerror
return fields.project, fields.location
And update the constructor with the something like:
def __init__(self, project: Optional, location: Optional, creds: Optional, resource_name_or_id: Optional[str]):
if resource_name_or_id:
project, location = self._get_and_validate_project_location(project, location, resource_name_or_id)
...
And update subclass call sites to pass in the resource noun in the constructor.
3907e4e
to
2e02b75
Compare
2e02b75
to
7a158ee
Compare
7a158ee
to
98612c2
Compare
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 Morgan!
unit tests:
Fixes <b/180923329>