-
Notifications
You must be signed in to change notification settings - Fork 141
adding pascal case to status #1213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
adding pascal case to status #1213
Conversation
| ) | ||
|
|
||
| @property | ||
| def to_pascal_case(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrmm, even though it's the case that this enumerate returned from describe/list happens to have the same values in a different format as the search attribute, not sure we want to guarantee that will always be the case. We intentionally document the explicit set of enumerate strings at https://docs.temporal.io/search-attribute#default-search-attribute without reference to this enumerate, but it's technically possible that list could change unrelated to this enumerate (even if it happens to match today even in server code). Most (all?) parts of the list filter/query don't have non-string-literal representations in SDKs at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understandable, would it make more sense to have something as part of the search attributes then?
That way, you would not introduce a breaking change if you change the format and update the property/function? Just thought it seemed strange that your own types would not work as search attributes out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a couple of search attributes that have pre-defined enumerates (e.g. TemporalWorkflowVersioningBehavior in addition to this one), but the actual type is truly Keyword which is a string. I think, in the absence of a type safe query builder or even a predefined set of SearchAttributeKeys for all of the built-in search attributes, we currently recommend using the string format only for keys/values for list filters based on expectations in the doc instead of hardcoded SDK-side expectations.
We can keep an issue open for if/when we do decide to provide type safe assistance to building of list filters (not just enumerates, but the predefined typed key set, TemporalWorkerDeploymentVersion parsing, conditional/query building, string escaping, etc). And we would of course do it across every SDK for parity not just Python.
What was changed
Added a
to_pascal_caseproperty to theWorkflowExecutionStatusenum class that converts enum names to PascalCase format required by Temporal search attributes. The property converts enum names likeTIMED_OUTto"TimedOut"andCONTINUED_AS_NEWto"ContinuedAsNew".Why?
Temporal search attributes expect execution status values in PascalCase format without underscores. Previously, users had to manually convert enum names when filtering workflows by execution status using search attributes. This property eliminates the need for manual conversion and provides a convenient, consistent way to get the correctly formatted string.
Example usage:
Checklist
Closes Add pascalcase to WorkflowExecutionStatus #1212
How was this tested:
Added unit test
test_workflow_execution_status_search_attribute_value()intests/test_client.pythat verifies correct PascalCase conversion for allWorkflowExecutionStatusenum values:RUNNING→"Running"COMPLETED→"Completed"FAILED→"Failed"CANCELED→"Canceled"TERMINATED→"Terminated"TIMED_OUT→"TimedOut"CONTINUED_AS_NEW→"ContinuedAsNew"The test can be run with:
No documentation updates needed. This is a straightforward addition to an existing enum class that follows standard Python property patterns.