Update schedule search attributes #753
Conversation
dandavison
force-pushed
the
schedule-update-sa
branch
2 times, most recently
from
4d105ff to
a3b5856
Compare
| and len(desc.search_attributes) == 0 | ||
| ) | ||
|
|
||
| await assert_eq_eventually(True, expectation) |
There was a problem hiding this comment.
This is the test case that fails without the change above relating to empty map in the request proto.
dandavison
force-pushed
the
schedule-update-sa
branch
from
a3b5856 to
2fd720f
Compare
| request_id=str(uuid.uuid4()), | ||
| ) | ||
| if update.search_attributes is not None: | ||
| request.search_attributes.indexed_fields.clear() # Ensure that we at least create an empty map |
There was a problem hiding this comment.
Without this, if the user sets an empty but non-None value, then the request proto does not contain a search_attributes field at all. That's incorrect; what we want in that case is an empty map, which clears all SAs server-side. See docs in API: temporalio/api:/temporal/api/workflowservice/v1/request_response.proto
I investigated making this change (empty map in proto for empty but non-None SAs) in encode_search_attributes so that it is applied consistently to all code paths, but it causes the existing test test_schedule_workflow_search_attribute_update to break.
There was a problem hiding this comment.
This approach makes sense since, uniquely in Python, we pass the collection to write to for the helpers instead of just assign the variable due to how Python protos work. Though it is strange you can't just start writing to the map and it is created lazily.
| request_id=str(uuid.uuid4()), | ||
| ) | ||
| if update.search_attributes is not None: | ||
| request.search_attributes.indexed_fields.clear() # Ensure that we at least create an empty map |
There was a problem hiding this comment.
This approach makes sense since, uniquely in Python, we pass the collection to write to for the helpers instead of just assign the variable due to how Python protos work. Though it is strange you can't just start writing to the map and it is created lazily.
Run tests sequentially and fix reference to workflow.NondeterminismError
Fixes #594
Support updating schedule SAs. The API is that the user sets typed SAs in the same "updater" function that they use for updating the schedule itself.