Search attributes #43
Conversation
# Conflicts: # tests/worker/test_workflow.py
# Conflicts: # .github/workflows/ci.yml # README.md # temporalio/worker/workflow.py # temporalio/workflow.py # tests/worker/test_workflow.py
| ) -> None: | ||
| """Encode search attributes as bridge payloads.""" | ||
| for k, vals in attrs.items(): | ||
| payloads[k].CopyFrom( |
There was a problem hiding this comment.
Does this create the key k if it doesn't exist?
There was a problem hiding this comment.
Or do you always make sure it exists before sending to this method?
There was a problem hiding this comment.
This is a Protobuf oddity. They lazily create everything on first scalar (i.e. non-message) update. See https://developers.google.com/protocol-buffers/docs/reference/python-generated#map-fields. If I tried to do payloads[k] = some_message, I'd get an error because no Python Protobuf code allows assignment to a message.
| raise TypeError("Search attribute values must be lists") | ||
| # Convert dates to strings | ||
| safe_vals = [] | ||
| for v in vals: |
There was a problem hiding this comment.
You should assert that vals is a homogenous list
There was a problem hiding this comment.
Could also limit this at the type level.
We should have done the same in TS @lorensr (still not too late).
type SearchAttributeValueArray = string[] | number[] | boolean[] | Date[];
type SearchAttributes = Record<string, SearchAttributeVAlueArray>;
There was a problem hiding this comment.
I am afraid that Union[List[str], List[int], List[float], List[bool], List[datetime]] doesn't work as we'd like. You can't assign an empty list to it for example (this is a known MyPy and Python typing issue, see issue 3283 in https://github.com/python/mypy). We're gonna have to keep the types as is and only do runtime checks.
@bergundy - I think we need more details on what should be in the describe response. Right now I don't surface anything but the status and the raw gRPC message (so they can decode if they want). Why did TypeScript make |
Don't really see a reason why not same for |
What was changed
Checklist