Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Save CheckpointUri in NodeExecution.Closure #479

Merged
merged 3 commits into from
Oct 6, 2022

Conversation

andrewwdye
Copy link
Contributor

Signed-off-by: Andrew Dye [email protected]

TL;DR

This changes saves the checkpoint path in the NodeExecutionModel so that we have it when recovering a workflow. Previously intra task checkpointing was only supported for task level retries within a single node execution.

Related PR: flyteorg/flytepropeller#486

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

This change adds CheckpointUri to the NodeExecutionModel stored in the admin db, as part of TaskNodeMetadata in the Closure.

Testing

See flyteorg/flytepropeller#486 for end-to-end testing strategy

Tracking Issue

flyteorg/flyte#2254

Follow-up issue

NA

Signed-off-by: Andrew Dye <[email protected]>
@codecov
Copy link

codecov bot commented Sep 26, 2022

Codecov Report

Merging #479 (15eb707) into master (c09eb7d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #479      +/-   ##
==========================================
+ Coverage   61.55%   61.61%   +0.05%     
==========================================
  Files         158      158              
  Lines       11490    11492       +2     
==========================================
+ Hits         7073     7081       +8     
+ Misses       3683     3677       -6     
  Partials      734      734              
Flag Coverage Δ
unittests 60.55% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/repositories/transformers/node_execution.go 72.68% <100.00%> (+0.28%) ⬆️
...implementations/workflow_execution_event_writer.go 80.00% <0.00%> (+40.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@hamersaw hamersaw merged commit d1ebaae into flyteorg:master Oct 6, 2022
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Save CheckpointUri in NodeExecution.Closure

Signed-off-by: Andrew Dye <[email protected]>

* Test failures

Signed-off-by: Andrew Dye <[email protected]>

Signed-off-by: Andrew Dye <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants