temporalio / documentation

Temporal documentation
https://docs.temporal.io
Other
85 stars 224 forks source link

SDK-1486: Python update docs #2976

Closed dandavison closed 2 months ago

dandavison commented 2 months ago

[rendered]

This is a new version of the message-passing documentation for Python. It follows on from the new top-level section on https://docs.temporal.io/encyclopedia/workflow-message-passing that @drewhoskins-temporal recently wrote. The significant changes here are:

The structure of the document is (I'll keep this up-to-date it it changes)

## Writing message handlers
  ### Query handlers {#handle-query}
  ### Signal handlers {#handle-signal}
  ### Update handlers and validators {#handle-update}

## Sending messages
  ### Sending a Query {#send-query}
  ### Sending a Signal
    #### Sending a Signal from a Client {#send-signal-from-client}
    #### Sending a Signal from a Workflow {#send-signal-from-workflow}
    #### Signal-With-Start {#signal-with-start}
  ### Sending an Update {#send-update}
    #### 1. Use [`execute_update`](https://python.temporal.io/temporalio.client.WorkflowHandle.html#execute_update) to wait for the update to complete
    #### 2. Use [`start_update`](https://python.temporal.io/temporalio.client.WorkflowHandle.html#start_update) to receive a handle as soon as the update is accepted or rejected
    #### Non-type safe APIs

## Message handler patterns {#message-handler-patterns}
  ### Async handlers {#async-handlers}
    #### Waiting
    #### Use `asyncio.Lock` to prevent concurrent handler execution {#control-handler-concurrency}
    #### Finishing handlers before the Workflow completes {#wait-for-message-handlers}

## Troubleshooting
  ### Problems when sending a Signal
  ### Problems when sending an Update
  ### Problems when sending a Query
## Dynamic Handlers {#dynamic-handler}
  ### Set a Dynamic Signal, Query, or Update handler {#set-a-dynamic-signal}
  ### Set a Dynamic Workflow {#set-a-dynamic-workflow}
  ### Set a Dynamic Activity {#set-a-dynamic-activity}

TODO:

axfelix commented 2 months ago

I'd like to see the introduction to this page stick slightly closer to the other pages in our SDK docs as right now I think it contrasts too much. Otherwise, though, I like the emphasis on implementation details that increases toward the end of the page, I like what you're doing, and I think this is pretty close to being able to be merged. I left a few more comments, but I don't think this needs another round of feedback from me. Will let others finish weighing in.

dandavison commented 2 months ago

Thanks for reviewing @axfelix.

I'd like to see the introduction to this page stick slightly closer to the other pages in our SDK docs as right now I think it contrasts too much.

I agree; I noticed this morning that the intro needed a bit more. Sorry it's been a bit of an evolving target during review due to our product launch aims.

fairlydurable commented 2 months ago

@drewhoskins-temporal Is this ready to transition from SDK-1486 to EDU-671?

dandavison commented 2 months ago

@drewhoskins-temporal Is this ready to transition from SDK-1486 to EDU-671?

Not quite -- I'll let you know when I've addressed everything remaining that I know of from my side.

dandavison commented 2 months ago

Is this ready to transition from SDK-1486 to EDU-671?

Yes, over to docs team!

dandavison commented 2 months ago

Erica's changes are merged into this one; this PR is now just waiting for an approval from docs team.

dandavison commented 2 months ago

Thanks @axfelix and @fairlydurable.

I've added two final commits for @fairlydurable to review:

  1. changing a code snippet only to address an uresolved review comment above: https://github.com/temporalio/documentation/pull/2976/commits/2820990b7ebd45ccdb5b72dfe70349c286d0c655

  2. Adding a line documenting workflow.current_update_info which should be included but was not: https://github.com/temporalio/documentation/commit/6446f01d78293aa42aa3be6766b8bc752ee1c11b

I'll obviously let you know if I think of any more omissions, but other than that over to you for merge.