-
Notifications
You must be signed in to change notification settings - Fork 76
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
Workflow Update #400
Merged
Merged
Workflow Update #400
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
e839219
Update Core
Sushisource cf7cc9b
Add docker image for proto building
Sushisource 2b0d6f0
Basic happy path working
Sushisource c2281c5
Validator running
Sushisource 84b909c
Add UpdateHandle. Polling not yet implemented.
Sushisource d1e0681
Add polling
Sushisource 713c847
Linting / mypy
Sushisource abe36c3
Update core / additional failure path tests
Sushisource 533b9f7
Dynamic update handlers
Sushisource 5daec88
Use class for update decorator
Sushisource 668532d
Add update definitions to workflow definition test
Sushisource 3a9dc2c
Finally! Type system defeated!
Sushisource 6a7c52a
Rename update to execute_update and hide wait_for_stage param
Sushisource 4be97cc
Always run validator interceptor
Sushisource 953b8e4
Clean up some last TODOs / workflow definition error tests
Sushisource 2e54df6
3.7 throws this error differently
Sushisource 188bf97
Update README
Sushisource 03e3197
Skip updates in Java test server for now
Sushisource cc0871d
Make validator interceptor sync
Sushisource de98531
Address various review comments
Sushisource afc1807
Update core & add inbound tracing interceptor
Sushisource 8b41042
Make sure only temporal errors fail update during handler
Sushisource 5d2b614
Merge branch 'main' into workflow-update
Sushisource 6b0972d
Renames / no poll update tracing
Sushisource 020f663
Add start_update overloads / update handle type
Sushisource 6670ee5
Ensure otel test will wait until after task fails to send update
Sushisource 1731fcf
Latest review comments
Sushisource 1c18fc2
Fix failure setting on update rejection
Sushisource fabaecf
Merge branch 'main' into workflow-update
Sushisource File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -458,8 +458,8 @@ Some things to note about the above code: | |||||
#### Definition | ||||||
|
||||||
Workflows are defined as classes decorated with `@workflow.defn`. The method invoked for the workflow is decorated with | ||||||
`@workflow.run`. Methods for signals and queries are decorated with `@workflow.signal` and `@workflow.query` | ||||||
respectively. Here's an example of a workflow: | ||||||
`@workflow.run`. Methods for signals, queries, and updates are decorated with `@workflow.signal`, `@workflow.query` | ||||||
and `@workflow.update` respectively. Here's an example of a workflow: | ||||||
|
||||||
```python | ||||||
import asyncio | ||||||
|
@@ -515,6 +515,12 @@ class GreetingWorkflow: | |||||
@workflow.query | ||||||
def current_greeting(self) -> str: | ||||||
return self._current_greeting | ||||||
|
||||||
@workflow.update | ||||||
def set_and_get_greeting(self, greeting: str) -> str: | ||||||
old = self._current_greeting | ||||||
self._current_greeting = greeting | ||||||
return old | ||||||
|
||||||
``` | ||||||
|
||||||
|
@@ -582,6 +588,14 @@ Here are the decorators that can be applied: | |||||
* All the same constraints as `@workflow.signal` but should return a value | ||||||
* Should not be `async` | ||||||
* Temporal queries should never mutate anything in the workflow or call any calls that would mutate the workflow | ||||||
* `@workflow.update` - Defines a method as an update | ||||||
* May both accept as input and return a value | ||||||
* May be `async` or non-`async` | ||||||
* May mutate workflow state, and make calls to other workflow APIs like starting activities, etc. | ||||||
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.
Suggested change
(pedantic, can ignore) |
||||||
* Also accepts the `name` and `dynamic` parameters like signals and queries, with the same semantics. | ||||||
* Update handlers may optionally define a validator method by decorating it with `@update_handler_method.validator`. | ||||||
To reject an update before any events are written to history, throw an exception in a validator. Validators cannot | ||||||
be `async`, cannot mutate workflow state, and return nothing. | ||||||
|
||||||
#### Running | ||||||
|
||||||
|
@@ -1440,6 +1454,13 @@ to `1` prior to running tests. | |||||
Do not commit `poetry.lock` or `pyproject.toml` changes. To go back from this downgrade, restore `pyproject.toml` and | ||||||
run `poetry update protobuf grpcio-tools`. | ||||||
|
||||||
For a less system-intrusive approach, you can: | ||||||
```shell | ||||||
docker build -f scripts/_proto/Dockerfile . | ||||||
docker run -v "${PWD}/temporalio/api:/api_new" -v "${PWD}/temporalio/bridge/proto:/bridge_new" <just built image sha> | ||||||
poe format | ||||||
``` | ||||||
|
||||||
### Style | ||||||
|
||||||
* Mostly [Google Style Guide](https://google.github.io/styleguide/pyguide.html). Notable exceptions: | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
FROM python:3.10 | ||
|
||
RUN python -m pip install --upgrade wheel "poetry==1.3.2" poethepoet | ||
VOLUME ["/api_new", "/bridge_new"] | ||
|
||
COPY ./ ./ | ||
|
||
RUN mkdir -p ./temporalio/api | ||
RUN poetry install --no-root -E opentelemetry | ||
RUN poetry add "protobuf<4" | ||
RUN poe gen-protos | ||
|
||
CMD cp -r ./temporalio/api/* /api_new && cp -r ./temporalio/bridge/proto/* /bridge_new |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
.git/ | ||
.idea/ | ||
.mypy_cache/ | ||
.pytest_cache/ | ||
.venv/ | ||
build/ | ||
dist/ | ||
temporalio/api/ | ||
temporalio/bridge/**/target/ | ||
temporalio/bridge/**/*.so | ||
Dockerfile |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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, I wonder if just altering
update_salutation
to be an update that return the previous salutation part would be enough. Also, I wonder if calling thisupdate_greeting
is descriptive enough. I don't think it matters much. But I will take the approach of return-old-thing in the .NET PR.