Prototype per-run DoNotCache task results - #2
Draft
wolever-gl wants to merge 1 commit into
Draft
wolever-gl wants to merge 1 commit into
wolever-gl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 495cd61. Configure here.
| # An explicit never-persisted record must not be advertised | ||
| # either; keep its unwrapped result as the terminal data. | ||
| payload = payload.result | ||
| terminal_state.data = payload |
There was a problem hiding this comment.
None results break local state retrieval
Medium Severity
When the unwrapped value is None, terminal_state.data is set to None. Local state.result() treats data is None as a missing persisted record and raises MissingResult, so return_state=True fails for an in-process DoNotCache(None) even though the direct caller still receives None.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 495cd61. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Overview
This is an exploratory prototype only, opened as a draft for design evidence. It is not an approval to merge or deploy, and it does not close any ticket.
Reference: RD-276 (Linear). This PR intentionally does not say "closes RD-276" — the design is under evaluation, not accepted.
Summary
Adds a public
DoNotCache(value)return wrapper that a task can use to opt a single run out of result caching and persistence at return time:DoNotCache(value), the task still completes in theCompletedstate and the direct caller still receives the raw value — but no result record is persisted and no cache entry is written under the transaction key.Cachedbehavior for tasks that do not opt out is unchanged.src/prefect/utilities/annotations.py(publicDoNotCacheannotation), unwrapped in both the sync and asynchandle_successpaths insrc/prefect/task_engine.py, re-exported throughsrc/prefect/main.py/src/prefect/__init__.py, with type overloads insrc/prefect/tasks.pysotaskfunctions returningDoNotCache[R]are typed asTask[..., R].Design notes
write_on_committransaction mechanism rather than introducing a new persistence switch.handle_successvariants unwrap the wrapper; this duplication is a known maintenance cost (see risks).Completedstate carries the raw value, but a state rehydrated through the API has no persisted record behind it and will surfaceMissingResult— acceptable for the prototype, a key design question for any real implementation.Tests
tests/test_task_engine.py: 183 passed, 2 xfailed (41.21s full run).TestDoNotCacheclass: 13 passed, covering sync/async opt-out, repeated execution on the same key, no persisted record on disk, commit hooks still firing, no advertised result metadata on Completed events, upstream-state wrapping, and explicit ResultRecord handling.Review / design risks
DoNotCachenever gets a chance to apply. Opt-out is therefore not a guarantee against earlier-cached results.MissingResultfor runs that opted out, since no record exists remotely.yieldedDoNotCachevalues are not unwrapped at runtime; only the terminalreturn DoNotCache(...)is. The type overloads encode this asymmetry.cache_policy.should_store(context, parameters, result), which is a policy-level alternative to a return-value annotation; any production design should be compared against it.Documentation
No documentation was added, intentionally: this is a prototype for design evidence. If the design proceeds toward a merge, the public API needs coverage in
docs/v3/concepts/caching.mdxanddocs/v3/concepts/results.mdx(both currently describe caching/persistence with no return-value opt-out mechanism) before it ships. This missing coverage is accepted only because this PR is a draft prototype.Documentation preflight (concrete evidence)
git diff --name-only c69ade89e0c42c4e98d9f2062d6cc64aeb256ed2...HEADtouches only:src/prefect/__init__.py,src/prefect/main.py,src/prefect/task_engine.py,src/prefect/tasks.py,src/prefect/utilities/annotations.py,tests/test_task_engine.py.DoNotCachefinds references only in those source/test files — no existing documentation references it, as expected.docs/v3/concepts/caching.mdx,docs/v3/concepts/results.mdx,docs/v3/how-to-guides/workflows/cache-workflow-steps.mdx(none mention a return-value opt-out today).AGENTS.md
Checked: no
AGENTS.mdupdate is needed — the change introduces no new commands, module structure, or architectural drift.Checklist
TestDoNotCachesuite; 183 passed, 2 xfailed).mint.json. (N/A — no docs files were removed.)DoNotCachecarries a docstring with a usage example).Note
Medium Risk
Touches the task success path and result/cache transaction commit behavior in both sync and async engines; opt-out does not apply when a cache hit occurs before the task runs, and API-rehydrated states lack persisted results.
Overview
Introduces a public
DoNotCache(value)annotation so a task can complete normally and return the unwrapped value while skipping cache writes and result persistence for that run.The sync and async
handle_successpaths unwrap the wrapper, settransaction.write_on_commit = False, and replace terminal state data so events/API payloads do not advertise storage keys for records that were never written; the state staysCompleted(notCached) on repeat runs with the same cache key.DoNotCacheis re-exported fromprefect;@taskoverloads (includingConfiguredTaskDecorator) type tasks that returnDoNotCache[R]asTask[..., R], with generator terminal returns handled separately from yielded wrappers.Adds
TestDoNotCachecovering repeated execution, disk persistence, hooks, events, and API state behavior. Draft prototype for RD-276—not documented for production yet.Reviewed by Cursor Bugbot for commit 495cd61. Bugbot is set up for automated code reviews on this repo. Configure here.