Skip to content
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

EDU-3229: Workflow Initializers Encyclopedia entry #3114

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

drewhoskins-temporal
Copy link
Contributor

What does this PR do?

Overview documentation the new Workflow Initializer feature.

Notes to reviewers

  • Per-SDK docs are TBD.
  • Plan is to link to this from my blog post on message handling.

Test plan:

yarn start
http://localhost:3000/encyclopedia/workflow-message-passing#workflow-initializers

Click on links.

@fairlydurable
Copy link
Contributor

fairlydurable commented Sep 30, 2024

@drewhoskins-temporal This is a wonderful expansion that reduces developer confusion and helps make sure that handlers aren't run before Workflow state is set.

Based on the Friday meeting, Docs/EDU should be providing guidance rather than re-writes. Happy to set up a Zoom call.

A few points of feedback

  1. Tone: Aiming for a formal but friendly/supportive tone with few complex words and short sentences. For instance, the phrase "So, it's important to initialize your Workflow's state" can be revised to "Always initialize Workflow state before registering message handlers." It's more formal, it avoids passive voice, and it's more specific.
  2. Context The start is abrupt. Spend a little more time introducing what's going on. A clear introduction explains the content purpose and importance. For example, start with a statement like, "Initializing Workflow state helps applications handle incoming messages correctly." (with your words of course.) This sets the stage for the discussion from the outset. Then, try to expand with motivation of what they'll get from this section. "Under normal circumstances, your messages might be processed before a Workflow's main method runs for the first time. You can see this in the Message Handler flow shown in the concurrency diagram on this page. In practice, this often happens in two scenarios:" (again, your words)
  3. Construction: Avoid passive voice and minimize modal auxiliaries. If you have access to Open AI or similar, I can suggest some prompts that will tighten the text so you don't have to do it by hand, saving time. The other two points need planning and thought, but this one really doesn't.

Thank you for filing the ticket and getting this update underway. It's appreciated.

Also, always feel free to reach out to me to help me improve my feedback. As we're growing our cooperation I want to find ways to best support these items.

@fairlydurable fairlydurable changed the title Workflow Initializers Encyclopedia entry EDU-3229: Workflow Initializers Encyclopedia entry Sep 30, 2024
docs/encyclopedia/application-message-passing.mdx Outdated Show resolved Hide resolved
docs/encyclopedia/application-message-passing.mdx Outdated Show resolved Hide resolved
docs/encyclopedia/application-message-passing.mdx Outdated Show resolved Hide resolved
- Finishing handlers before the Workflow completes.
- Ensuring your messages are processed exactly once.
- Injecting work into the main Workflow
- Finishing handlers before the Workflow completes
Copy link
Contributor

Choose a reason for hiding this comment

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

Only because of previous tickets do I know that this really means something along the lines of blocking the Workflow's completion until all handlers have finished processing, otherwise I don't know if it really would be meaningful to me at first read.

Suggest minor rewording to make it clearer

- Ensuring your messages are processed exactly once.
- Injecting work into the main Workflow
- Finishing handlers before the Workflow completes
- Ensuring your messages are processed exactly once
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very cool. Whose responsibility is this? Is it worth a sidebar ("admonition"/callout) with a link to a source of truth to explain? Does this happen often?

@@ -220,22 +220,40 @@ Every time the Workflow wakes up--generally, it wakes up when it needs to--it wi

This execution is on a single thread–while this means you don’t have to worry about parallelism, you do need to worry about concurrency if you have written Signal and Update handlers that can block. These can run interleaved with the main Workflow and with one another, resulting in potential race conditions. These methods should be made reentrant.

#### Initializing the Workflow first {#workflow-initializers}

Initialize your Workflow's state before handling messages. This prevents your Workflow reading uninitialized instance variables.
Copy link
Contributor

@fairlydurable fairlydurable Oct 3, 2024

Choose a reason for hiding this comment

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

I think you mean that it prevents your Handlers from reading uninitialized instance variables.

Suggested change
Initialize your Workflow's state before handling messages. This prevents your Workflow reading uninitialized instance variables.
Initialize your Workflow's state before handling messages.
This prevents your Handlers from reading uninitialized instance variables if they arrive before the Workflow state is entire set up.

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.

3 participants