-
Notifications
You must be signed in to change notification settings - Fork 104
Access current update info with ID inside update handler #544
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
Changes from all commits
4de7737
4a299a5
3bb76e6
593d950
d2d3b93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
from __future__ import annotations | ||
|
||
import asyncio | ||
import contextvars | ||
import inspect | ||
import logging | ||
import threading | ||
|
@@ -424,6 +425,17 @@ class ParentInfo: | |
workflow_id: str | ||
|
||
|
||
@dataclass(frozen=True) | ||
class UpdateInfo: | ||
"""Information about a workflow update.""" | ||
|
||
id: str | ||
"""Update ID.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stray string (should use comment, not string, it it's explaining the field). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For dataclasses we have use docstrings like this in the past. For example, see https://python.temporal.io/temporalio.client.WorkflowExecution.html. |
||
|
||
cretz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: str | ||
"""Update type name.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This meaning is unclear to me. Is name the function name (unscoped by the class name) unless it's overridden with the attribute? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This is also used in the decorator and the client calls. The meaning of update ID above it may be unclear to users too. The "update name" is a Temporal concept like "workflow type" in "Info.workflow_type". Ideally the general documentation would detail this, but if necessary we can update all of the Python docs for signal/update/query name, all the workflow info stuff, and all of that if we want. But this one field isn't unique (it applies to the field above it, where "name" is used elsewhere, all the other fields we delegate to docs to describe, etc). |
||
|
||
|
||
class _Runtime(ABC): | ||
@staticmethod | ||
def current() -> _Runtime: | ||
|
@@ -654,6 +666,31 @@ async def workflow_wait_condition( | |
... | ||
|
||
|
||
_current_update_info: contextvars.ContextVar[UpdateInfo] = contextvars.ContextVar( | ||
"__temporal_current_update_info" | ||
) | ||
|
||
|
||
def _set_current_update_info(info: UpdateInfo) -> None: | ||
_current_update_info.set(info) | ||
|
||
|
||
def current_update_info() -> Optional[UpdateInfo]: | ||
"""Info for the current update if any. | ||
|
||
This is powered by :py:mod:`contextvars` so it is only valid within the | ||
update handler and coroutines/tasks it has started. | ||
|
||
.. warning:: | ||
This API is experimental | ||
|
||
Returns: | ||
Info for the current update handler the code calling this is executing | ||
within if any. | ||
""" | ||
return _current_update_info.get(None) | ||
|
||
|
||
def deprecate_patch(id: str) -> None: | ||
"""Mark a patch as deprecated. | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
We use
workflow_id
to differentiate from all the other_id
fields inInfo
, should we call thisupdate_id
? A simpleid
I think works here but can change if needed.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.
In Go we did ID, but
update_id
is also fine.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.
👍 I think
id
works best here.