Support new API fields in ActivityHandle.describe - #1782
Conversation
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| @dataclass(eq=False, kw_only=True) |
There was a problem hiding this comment.
Why are you unfreezing the dataclasses?
There was a problem hiding this comment.
Mostly to allow swapping out data converter, but maybe that's wrong design. I can see the argument to keep these frozen, I'll re-add it.
| info=resp.info, | ||
| long_poll_token=resp.long_poll_token or None, | ||
| # Erase unrequested fields if server sent them anyway (can happen with old server) | ||
| if not input.include_input: |
There was a problem hiding this comment.
This seems a little sketchy
There was a problem hiding this comment.
Currently (or until recently), the server sends heartbeat details and last failure unconditionally. This piece of code ensures the user has consistent view of data even if the server doesn't respect the default/false values in the request. One alternative is to keep the data in the proto but persist the include_ args in the description class and filter these out in the getters. Another is to just allow the possibility that these fields may be set anyway even if not requested. Of the three, I think erasing it from the proto is the best approach.
2 participants
What was changed
@dataclassconfiguration for ActivityExecution and ActivityExecutionDescriptionWhy?
Implements newly added API features and removes parts that were not meant to be exposed.
Checklist
Updated
tests.test_activity.TestDescribe