feat(py): require a live Operation on check and cancel - #6131
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the background operation polling and cancellation mechanisms to accept both live Operation objects and raw dictionary mappings (persisted poll handles), ignoring extra keys like latencyMs to prevent failures during reload. It also introduces more robust error handling using GenkitError and adds comprehensive unit tests. The review feedback suggests two performance optimizations: avoiding redundant serialization/re-validation when the input is already an Operation instance, and precomputing the set of valid operation fields to avoid linear scans in is_operation_field.
a9d6a64 to
e685b5f
Compare
e685b5f to
c726ab6
Compare
12a19b4 to
45e341a
Compare
0e0d436 to
5af4013
Compare
0d02936 to
705c1d4
Compare
705c1d4 to
89b7f9a
Compare
Summary
check_operation/cancel_operationtake a liveOperation. A persist dump isOperation.model_validate(dumped)first. Extra keys (latencyMs) are ignored so that validate does not 500.Operation(extra='ignore'). NotOperation | dict.ModelResponse, or astrisINVALID_ARGUMENT(notAttributeError). Messages:got a dump; pass Operation.model_validate(...),got ModelResponse; pass response.operation,got str, expected Operation.BackgroundAction.check/BackgroundAction.cancel, so a dump cannotAttributeErroron.actionbeforeUNIMPLEMENTED.UNIMPLEMENTEDon both the veneer and a realBackgroundAction.cancel. Unknown action isINVALID_ARGUMENT.