-
Notifications
You must be signed in to change notification settings - Fork 2k
Clarify result_storage usage with @task and @flow decorators
#19671
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
Conversation
- Add Warning callout to results.mdx explaining block instances vs string references - Update @flow and @task decorator docstrings to document both patterns - Improve error messages to suggest string reference alternative Closes #16641 Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
CodSpeed Performance ReportMerging #19671 will not alter performanceComparing Summary
|
docs/v3/advanced/results.mdx
Outdated
|
|
||
| When specifying `result_storage` in `@flow` or `@task` decorators, you have two options: | ||
|
|
||
| - **Block instances**: Must call `.save()` first, since decorators execute at import time before a server connection may be available |
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.
| - **Block instances**: Must call `.save()` first, since decorators execute at import time before a server connection may be available | |
| - **Block instances**: The block instance must be saved server-side or loaded from a saved block instance before it is provided to the `@task` or `@flow` decorator. |
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.
Applied both suggestions - thanks for the feedback!
docs/v3/advanced/results.mdx
Outdated
| - **Block instances**: Must call `.save()` first, since decorators execute at import time before a server connection may be available | ||
| - **String references**: Use the format `"block-type-slug/block-name"` for deferred resolution at runtime | ||
|
|
||
| For testing scenarios (especially with pytest), string references are recommended since they don't require server connectivity at import time. |
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.
| For testing scenarios (especially with pytest), string references are recommended since they don't require server connectivity at import time. | |
| For testing scenarios, string references are recommended since they don't require server connectivity at import time. |
result_storage usage with @task and @flow decorators
- Update block instances description to be clearer - Remove pytest parenthetical from testing scenarios Co-Authored-By: [email protected] <[email protected]>
docs/v3/advanced/results.mdx
Outdated
| other_storage_task() | ||
| ``` | ||
|
|
||
| <Warning> |
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.
Can you make this an Info admonition instead of a Warning?
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.
Done - changed to Info admonition.
Addresses common user confusion around
@flowand@taskdecorators when usingresult_storagewith unsaved block instances, especially in pytest scenarios.Changes
Documentation (docs/v3/advanced/results.mdx):
result_storage:.save()first)Docstrings (flows.py, tasks.py):
result_storageparameter documentation in@flowand@taskdecorators to explain both patterns and when to use eachError messages:
Closes #16641
Checklist
<link to issue>"mint.json.Review checklist
from prefect.filesystems import LocalFileSystemis correct in the documentation exampleLink to Devin run: https://app.devin.ai/sessions/8a4d5455065443f2ac1d44b4bd4664fc
Requested by: [email protected] (@desertaxle)