-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Updates caching docs to be all about the new style of caching #14165
Conversation
- the code definition of the task | ||
- the prevailing flow run ID, or if executed autonomously, the prevailing task run ID | ||
|
||
All of these are hashed to compute the task's _cache key_. |
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.
I think it is important to acknowledge the expected consequence is that a task called twice in the same flow with the same arguments will not run twice but reuse its cached results -- inter-flow idempotency.
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.
Good call, will do
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.
@jlowin updated
- `DEFAULT`: this cache policy uses the task's inputs, it's code definition, as well as the prevailing flow run ID | ||
to compute the task's cache key. | ||
- `INPUTS`: this cache policy uses _only_ the task's inputs to compute the cache key. | ||
- `TASKDEF`: this cache policy uses _only_ the task's code definition to compute the cache key. |
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.
FWIW, all the other values here are self documenting but this one really isn't clear unless you read this doc.
Suggestions:
TASK_CODE
TASK_DEF
(maybe)
TASK_SOURCE
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.
Love it; TASK_SOURCE
sounds best to me but it's a bit formal. Will need to think about which to use but I'll change it either way
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.
source actually hints at how I get this (getsource()
) so I think it's the best
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.
Made this change in my code companion PR to this: e8c996d
Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
Note that some changes depend on #14164
My goal here was to give a practical orientation to caching without mentioning transactions, as those will be more advanced