-
Notifications
You must be signed in to change notification settings - Fork 279
Added the plugin guide #3990
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
base: main
Are you sure you want to change the base?
Added the plugin guide #3990
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📖 Docs PR preview links
|
docs/develop/plugins-guide.mdx
Outdated
| If you prefer to learn by getting hands-on with code, check out some existing plugins. | ||
|
|
||
| - Temporal's Python SDK ships with an [OpenAI Agents SDK](https://github.com/temporalio/sdk-python/tree/main/temporalio/contrib/openai_agents) plugin | ||
| - Pydantic plugin when it’s fixed |
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.
@drewhoskins-temporal Do you know when this will be ready or should I just leave it out for now?
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.
They have implemented it, but checking it in is blocked on us releasing the python SDK, which has been delayed. (So is this PR, btw!)
Let's leave it out and I'll make sure to flag when it's ready again.
drewhoskins-temporal
left a comment
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.
Thank you! I made a few suggested edits and tagged Tim in for a few.
(Bearing in mind that in some cases I was editing my own content. :-) )
docs/develop/plugins-guide.mdx
Outdated
| Temporal's Activity retry mechanism gives applications the benefits of durable execution. | ||
| For example, Temporal will keep track of the exponential backoff delay even if the Worker crashes. Since Temporal can’t tell when a Worker crashes, Workflows rely on the [start_to_close_timeout](https://docs.temporal.io/encyclopedia/detecting-activity-failures#start-to-close-timeout) to know how long to wait before assuming that an Activity is inactive. | ||
|
|
||
| Be cautious when doing retries within your Activity because it lengthens this overall timeout to be longer than the longest possible retry sequence. That means it takes too long to recover from other causes of failure. Such internal retries also prevent users from counting failure metrics and make it harder for users to debug in Temporal UI when something is wrong. |
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.
This ended up feeling a bit long and redundant on my re-read.
| Be cautious when doing retries within your Activity because it lengthens this overall timeout to be longer than the longest possible retry sequence. That means it takes too long to recover from other causes of failure. Such internal retries also prevent users from counting failure metrics and make it harder for users to debug in Temporal UI when something is wrong. | |
| Be cautious when doing retries within your Activity because it lengthens the needed Activity timeout. Such internal retries also prevent users from counting failure metrics and make it harder for users to debug in Temporal UI when something is wrong. |
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.
Same deal as my above comments, not sure this whole section even needs to be here at all. We're just repeating advice we give elsewhere in the docs - none of this is specific to plugins
| You can provide a library with functionality for use within a Workflow. Your library will call elements you include in your Plugin Activities, Child Workflows, Signals, Updates, Queries, Nexus Operations, Interceptors, Data Converters, and any other code as long as it follows these requirements: | ||
|
|
||
| - It should be [deterministic](https://docs.temporal.io/workflows#deterministic-constraints), running the same way every time it’s executed. Non-deterministic code should go in Activities or Nexus Operations. | ||
| - It should be used in the Python [sandbox](https://docs.temporal.io/develop/python/python-sdk-sandbox). |
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.
This is python-specific. Could we maybe do this in a tab? Or otherwise reformat into an SDK-specific section or something?
s/used/usable/
docs/develop/plugins-guide.mdx
Outdated
|
|
||
| - It should be [deterministic](https://docs.temporal.io/workflows#deterministic-constraints), running the same way every time it’s executed. Non-deterministic code should go in Activities or Nexus Operations. | ||
| - It should be used in the Python [sandbox](https://docs.temporal.io/develop/python/python-sdk-sandbox). | ||
| - It should be designed to handle being restarted, resumed, or executed independently of where or how it originally began without losing correctness or state consistency. |
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.
| - It should be designed to handle being restarted, resumed, or executed independently of where or how it originally began without losing correctness or state consistency. | |
| - It should be designed to handle being restarted, resumed, or executed in a different process from where it originally began without losing correctness or state consistency. |
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.
@tconley1428, can you provide details about how to test this by disabling the workflow cache, starting with python? My thought is we can add a "Testing your plugin" section at the bottom and then link to that here.
| - Observability, tracing, or logging middleware | ||
| - Adding reliable built-in functionality such as LLM calls, corporate messaging, and payments infrastructure | ||
| - Encryption or compliance middleware | ||
|
|
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.
Let's frame the guide:
This guide will
- introduce you to the mechanics of creating plugins
- provide general advice for platform engineers in harnessing and maintaining Temporal's primitives such as Workflows and Activities.
| async def run(self, name: str) -> str: | ||
| return f"Hello, {name}!" | ||
|
|
||
| plugin = SimplePlugin( |
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.
@tconley1428 my initial reaction was that this is unrealistic, since plugins are generally designed to be reused. I'd expect people to create a plugin class that declares the Workfows rather than make a global variable for people to use. If you agree, can you help update the code, here and in the other examples?
| # Should consider interactions with other plugins, | ||
| # as this will override the data converter. | ||
| # This may mean failing, warning, or something else |
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.
- This doesn't quite match what we did for OpenAI -- please double check that this is our best up-to-date advice.
- I can't find it, but I remember seeing a snippet where we pull the user's existing codecs into the payload. Should we do something like that here?
|
|
||
| # Plugins | ||
|
|
||
| A **Plugin** is an abstraction that will let your users add your feature to their Workflows in one line. It makes it seamless for platform developers to add their own custom functionality to many Workflows. Using plugins, you can build reusable open source libraries or build add-ons for engineers at your company. |
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.
Maybe re-word this to be more generic than adding stuff to workflows, since you can do more than that?
| - Single write operations | ||
| - Batches of similar writes | ||
| - One or more read operations followed by a write operation | ||
| - A read that should be memoized, like an LLM call, a large download, or a slow-polling read |
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.
This reads awkwardly to me. Each line sort of contradicts the last. Presumably we have advice elsewhere in the docs we can link to about what an activity should be scoped to?
This could be just "Activities should be scoped to a small, ideally idempotent, unit of work" or something
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.
Presumably we have advice elsewhere in the docs we can link to about what an activity should be scoped to?
We actually don't AFAICT! I'd love to link out to the general activity docs, but I figured, let's get the content in here for starters and then refactor next.
Let me suggest a rewording.
re: idempotent, we have to be careful there since LLMs are not idempotent by some definitions.
| - Activity arguments and return values must be serializable. | ||
| - Activities that perform writes should be idempotent. | ||
| - Activities have [timeouts](https://docs.temporal.io/develop/python/failure-detection#heartbeat-timeout) and [retry policies](https://docs.temporal.io/encyclopedia/retry-policies). To be Activity-friendly, your operation should either complete within a few minutes or it should support the ability to heartbeat or poll for a result. This way it will be clear to the Workflow when the Activity is still making progress. | ||
| - You need to specify at least one timeout, typically the [start_to_close_timeout](https://docs.temporal.io/encyclopedia/detecting-activity-failures#start-to-close-timeout). Keep in mind that the shorter the timeout, the faster Temporal will retry upon failure. See the [timeouts and retry policies](#timeouts-and-retry-policies) to learn more. |
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.
Definitely seems like we should link to general advice on activities somewhere in this section at least
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.
Agreed, but we don't actually have almost any of this advice anywhere that I could find. I've opened a task as a follow-up to figure out where to put it.
docs/develop/plugins-guide.mdx
Outdated
| Temporal's Activity retry mechanism gives applications the benefits of durable execution. | ||
| For example, Temporal will keep track of the exponential backoff delay even if the Worker crashes. Since Temporal can’t tell when a Worker crashes, Workflows rely on the [start_to_close_timeout](https://docs.temporal.io/encyclopedia/detecting-activity-failures#start-to-close-timeout) to know how long to wait before assuming that an Activity is inactive. | ||
|
|
||
| Be cautious when doing retries within your Activity because it lengthens this overall timeout to be longer than the longest possible retry sequence. That means it takes too long to recover from other causes of failure. Such internal retries also prevent users from counting failure metrics and make it harder for users to debug in Temporal UI when something is wrong. |
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.
Same deal as my above comments, not sure this whole section even needs to be here at all. We're just repeating advice we give elsewhere in the docs - none of this is specific to plugins
| </SdkTabs.Python> | ||
| </SdkTabs> | ||
|
|
||
| ### Workflow libraries |
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.
It's not clear what workflow "library" means here. Simply stating that it's some code you define on the plugin that users can call from their workflows would help. Or maybe a small code example too.
Co-authored-by: Spencer Judge <[email protected]>
Co-authored-by: Spencer Judge <[email protected]>
Co-authored-by: Spencer Judge <[email protected]>
Co-authored-by: Spencer Judge <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
Co-authored-by: Drew Hoskins <[email protected]>
What does this PR do?
This is blocked until the Python SDK release.
Notes to reviewers