feat: LRO handler#30
Conversation
sasha-gitg
left a comment
There was a problem hiding this comment.
High level comment, taking this LRO as an example: https://github.com/googleapis/python-aiplatform/blob/dev/google/cloud/aiplatform/models.py#L211
When we receive an LRO from the GAPIC API call, it's already initialized and polling so we may not want to initialize a separate LRO and have two LROs polling for the same result. Unless we want to delete the initial LRO which I think adds too much complexity for what we want.
Instead, I think we want a Class that takes the LRO initialized by the GAPIC library as an attribute, exposes methods that are relevant to the MB usage (result, add_done_callback).
Additionally, we have a requirement to be able to update a MB object with resource properties once the LRO is complete. In the example linked above that would look something like:
lro = api_client.upload_model(...)
mb_lro = LRO(lro)
empty_model = cls.init_from_lro(api_client, mb_lro)
return empty_modelThe init_from_lro on Model would look something like:
@classmethod
def init_from_lro(cls, api_client, lro):
self = cls.__new__(cls)
self.api_client = api_client
self.lro = lro
self.lro.add_update_resource_callback(
self, lro_result_key='model, api_get=lambda name: api_client.get_model(name=name) )
return selfSo the LRO class would look something like:
class LRO:
def __init__(self, gapic_lro: operation.Operation):
self._lro = gapic_lro
def add_update_resource_callback(self,
resource_noun_obj: base.AiPlatformResourceNoun,
result_key: str, api_get: Callable):
def callback(result):
result_value = getattr(result, result_key)
resource = api_get(result_value)
resource_noun_obj._gca_resource = resource
self._lro.add_done_callback(callback)…rm into long-running-op
| return operation_proto | ||
|
|
||
|
|
||
| def make_operation_future(client_operations_responses=None): |
There was a problem hiding this comment.
For later: move this into test fixtures.
|
adding a blocking version of resource updating |
dizcology
left a comment
There was a problem hiding this comment.
Please make sure the kokoro build passes before merging.
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
* first lro attempt * address comments * add test Co-authored-by: Yu-Han Liu <yuhanliu@google.com>
fixes b/169779287