Skip to content

Revamp python Continue-as-New documentation #3549

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

Merged
merged 17 commits into from
May 7, 2025
Merged

Revamp python Continue-as-New documentation #3549

merged 17 commits into from
May 7, 2025

Conversation

drewhoskins-temporal
Copy link
Contributor

@drewhoskins-temporal drewhoskins-temporal commented Apr 23, 2025

What does this PR do?

  • Introduces is_continue_as_new_suggested
  • Demonstrates how to test Continue-as-New
  • Explains how other scaling limits besides history size are relevant.
  • Raises awareness of all_handlers_finished

Test Plan

yarn start
http://localhost:3000/develop/python/continue-as-new

Click all the links

Notes to reviewers

@drewhoskins-temporal drewhoskins-temporal requested a review from a team as a code owner April 23, 2025 17:00
@drewhoskins-temporal drewhoskins-temporal changed the title Refresh python Continue-as-New documentation Revamp python Continue-as-New documentation Apr 23, 2025
[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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 None so 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. I did as you suggested, albeit mostly in the next section.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Contributor

@dandavison dandavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more suggestion but LGTM!

Copy link
Contributor

@brianmacdonald-temporal brianmacdonald-temporal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


### Considerations for Workflows with Message Handlers {#with-message-handlers}

> 3361c7e8 (Dan feedback)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing

@@ -65,12 +65,17 @@ An append-only log of [Events](#event) for your application.
- Event History is durably persisted by the Temporal service, enabling seamless recovery of your application state from crashes or failures.
- It also serves as an audit log for debugging.

**Event History limits**
### Event History limits {#event-history-limits}
Copy link
Contributor Author

@drewhoskins-temporal drewhoskins-temporal May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jsundai jsundai self-requested a review May 7, 2025 15:37
@jsundai
Copy link
Contributor

jsundai commented May 7, 2025

Added myself as a reviewer. Going to take a look today and can likely merge this today as well.

@jsundai jsundai merged commit 4948875 into main May 7, 2025
6 checks passed
@jsundai jsundai deleted the drewhoskins_CaN branch May 7, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants