Return None for SyncCheckpoint.read() when src is empty #1189
Conversation
This file contains 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
Signed-off-by: Andrew Dye <[email protected]>
andrewwdye
requested review from
wild-endeavor,
kumare3,
eapolinario and
pingsutw
as code owners
|
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov Report
@@ Coverage Diff @@
## master #1189 +/- ##
==========================================
+ Coverage 68.53% 68.55% +0.01%
==========================================
Files 288 288
Lines 26191 26201 +10
Branches 2925 2926 +1
==========================================
+ Hits 17951 17961 +10
Misses 7762 7762
Partials 478 478
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
kumare3
approved these changes
Sep 24, 2022
pingsutw
approved these changes
Sep 24, 2022
|
Congrats on merging your first pull request! 🎉 |
8 tasks
Ln11211
pushed a commit
to Ln11211/flytekit
that referenced
this pull request
Oct 2, 2022
Signed-off-by: Andrew Dye <[email protected]> Signed-off-by: Andrew Dye <[email protected]> Signed-off-by: LN <[email protected]>
eapolinario
added a commit
that referenced
this pull request
Oct 7, 2022
…mote.sync (#1212) * Open HashMethod to all types (#1171) * Open HashMethod to all types * Fix test in test_type_hints.py Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Signed-off-by: LN <[email protected]> * Return None for SyncCheckpoint.read() when src is empty (#1189) Signed-off-by: Andrew Dye <[email protected]> Signed-off-by: Andrew Dye <[email protected]> Signed-off-by: LN <[email protected]> * fix: plugins/flytekit-k8s-pod/requirements.txt to reduce vulnerabilities (#1192) The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-PROTOBUF-3031740 Signed-off-by: LN <[email protected]> * pyflyte non-fast register (#1205) * pyflyte run non-fast register Signed-off-by: Kevin Su <[email protected]> * lint Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> Signed-off-by: LN <[email protected]> * Update control_plane.rst Added about large response while using Remote.Sync() and how to handle it in the doc Signed-off-by: LN <[email protected]> * Update docs/source/design/control_plane.rst updating FlyteRemote sync message Co-authored-by: Samhita Alla <[email protected]> * Update control_plane.rst Updated the fix for the error and also removed the repeated error message line. Signed-off-by: LN <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: LN <[email protected]> Signed-off-by: Andrew Dye <[email protected]> Signed-off-by: Kevin Su <[email protected]> Signed-off-by: LN <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: Andrew Dye <[email protected]> Co-authored-by: Snyk bot <[email protected]> Co-authored-by: Kevin Su <[email protected]> Co-authored-by: Samhita Alla <[email protected]>
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.
Signed-off-by: Andrew Dye [email protected]
TL;DR
SyncCheckpoint.read()should returnNonewhen src path is empty rather than raising an exceptionType
Are all requirements met?
Complete description
Intra task checkpointing in Flyte passes a src path to the task in all attempts > 0. There is no guarantee that the prior attempt will have written checkpoint content.
SyncCheckpoint.read()should returnNonewhen src path is empty rather than raising an exception. Tasks must already handle this case, asNoneis returned on attempt 0.Tracking Issue
NA
Follow-up issue
NA