-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[Feature] Checkpointing with Workflows #17006
Conversation
After discussing with @masci, we landed on the following:
Will rework to this shape. |
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
359d07a
to
eaae6fa
Compare
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.
LGTM overall, left a couple of comments!
792c415
to
16f0b83
Compare
@@ -49,6 +49,7 @@ def __init__( | |||
) | |||
self._accepted_events: List[Tuple[str, str]] = [] | |||
self._retval: Any = None | |||
self._in_progress: Dict[str, List[Event]] = defaultdict(list) |
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.
We need to keep track of steps that are in progress at the creation time of a checkpoint. Otherwise, since their input events have already been popped from their respective queues, we will lose these events if we attempt to load from a checkpoint.
fbcf4b5
to
baf0ae8
Compare
Description
The additions/changes in this PR are made to enable a user to load and run a Workflow from a stored checkpoint from its past execution(s).
Round 4
Introduce
WorkflowCheckpointer
class that wraps aWorkflow
and creates and manages checkpoints.Workflow
as well asContext
)Context
: the UX gets a bit awkward because you need to pass context of previous runs along to maintain the checkpoint historyWorkflow
: then it becomes too attached to the instance -- you wouldn't be able to create/load checkpoints from different instances of the sameWorkflow
Sample Usage
Enabling/Disabling Steps For Checkpointing
Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Your pull-request will likely not be merged unless it is covered by some form of impactful unit testing.