Revamp python Continue-as-New documentation#3549
Conversation
| [Continue-As-New](/workflow-execution/continue-as-new) lets a Workflow Execution close successfully and create a new Workflow Execution. You can think of it as a checkpoint when your Workflow gets too long or approaches certain scaling limits. | ||
|
|
||
| The new Workflow Execution keeps the same Workflow Id but gets a new Run Id and a fresh Event History. | ||
| It also receives your Workflow's usual parameters. |
There was a problem hiding this comment.
How about a docs-prose-friendly version of the following?
You pass in the input to the new Workflow Run when you Continue-As-New. In practice what this means is that you will design your Workflow parameters so that you can pass in the "current state" when you Continue-As-New. You may want that parameter to default to
Noneso that you don't have to pass it when starting the Workflow normally.
Also I think a code sample showing a workflow definition that has been designed to work with Continue-As-New would be great here.
There was a problem hiding this comment.
Good suggestion. I did as you suggested, albeit mostly in the next section.
There was a problem hiding this comment.
One more suggestion: here, where you say this
lets a Workflow Execution close successfully and create a new Workflow Execution
I found it helpful to consult https://docs.temporal.io/workflow-execution#workflow-execution-chain where "close" and "Workflow Execution" are defined, and it's documented that indeed continue-as-new is one of the ways to close, and what a "Run ID" is.
So maybe a link from this point to there?
|
|
||
| The new Workflow Execution keeps the same Workflow Id but gets a new Run Id and a fresh Event History. | ||
| It also receives your Workflow's usual parameters. | ||
| Note that Continue-As-New works as an atomic operation. |
There was a problem hiding this comment.
I think this needs to be spelled out for users. You're saying either it closes and starts successfully, or doesn't close at all, right? Impossible to close and not start.
There was a problem hiding this comment.
On second thought, I think this is probably answering a question nobody has; I'm guessing they'd implicitly assume it was atomic. Removing it unless there's pushback.
| ``` | ||
|
|
||
| ::: | ||
| Check this helper each time you consider using Continue-as-New. |
There was a problem hiding this comment.
| Check this helper each time you consider using Continue-as-New. | |
| Check this helper in your test each time you consider using Continue-as-New. |
9658dc5 to
3361c7e
Compare
dandavison
left a comment
There was a problem hiding this comment.
One more suggestion but LGTM!
brianmacdonald-temporal
left a comment
There was a problem hiding this comment.
On the whole, this looks good. Concise, with the right level of detail, and I like the tone. I made a few suggestions, but nothing major.
|
|
||
| - When the Event History grows too large | ||
| - When the Workflow might get too many Updates or Signals | ||
|
|
There was a problem hiding this comment.
If possible, it would be good to elaborate on either of those conditions. is_continue_as_new_suggested() is a good tool, but readers may want to know what constitutes "too large" or "too many Updates or Signals."
These links might be useful:
https://docs.temporal.io/workflow-execution/event#time-constraints
https://docs.temporal.io/sending-messages#sending-signals
Co-authored-by: Brian MacDonald <brian.macdonald@temporal.io>
Co-authored-by: Brian MacDonald <brian.macdonald@temporal.io>
9ba7c94 to
b545fc2
Compare
|
|
||
| ### Considerations for Workflows with Message Handlers {#with-message-handlers} | ||
|
|
||
| > 3361c7e8 (Dan feedback) |
ecf773b to
9ba7c94
Compare
| - It also serves as an audit log for debugging. | ||
|
|
||
| **Event History limits** | ||
| ### Event History limits {#event-history-limits} |
There was a problem hiding this comment.
Note: this section is redundant with some cloud docs, https://docs.temporal.io/cloud/limits#per-workflow-execution-signal-limit , and also with https://docs.temporal.io/workflow-execution/event?_gl=1*9x2uv3*_gcl_au*MjU3MjQ3MTU2LjE3NDAxOTIxMjE.*_ga*MTE5NzU2NzkuMTcxOTQ1MzI2OA..*_ga_R90Q9SJD3D*czE3NDY1NjM0MDEkbzQ2JGcxJHQxNzQ2NTY2MTM2JGowJGwwJGgw#time-constraints
I don't want to link to the former because it's cloud specific.
Don't want to link to the latter because it's not canonical. It should go away or be cut down and just link back to this section.
Overall, this content needs some consolidation, but that's too much to take on in this PR.
|
Added myself as a reviewer. Going to take a look today and can likely merge this today as well. |
What does this PR do?
is_continue_as_new_suggestedall_handlers_finishedTest Plan
Click all the links
Notes to reviewers