This repository has been archived by the owner on Oct 9, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
Added overwrite parameter to catalog client.Put #285
Merged
hamersaw
merged 3 commits into
flyteorg:master
from
blackshark-ai:cache-eviction-update-artifact
Nov 4, 2022
Merged
Added overwrite parameter to catalog client.Put #285
hamersaw
merged 3 commits into
flyteorg:master
from
blackshark-ai:cache-eviction-update-artifact
Nov 4, 2022
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: Nick Müller <[email protected]>
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov Report
@@ Coverage Diff @@
## master #285 +/- ##
==========================================
+ Coverage 63.32% 63.37% +0.05%
==========================================
Files 145 145
Lines 9311 9324 +13
==========================================
+ Hits 5896 5909 +13
Misses 2872 2872
Partials 543 543
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This was referenced Oct 5, 2022
hamersaw
previously approved these changes
Nov 2, 2022
hamersaw
reviewed
Nov 2, 2022
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
hamersaw
approved these changes
Nov 4, 2022
Congrats on merging your first pull request! 🎉 |
eapolinario
pushed a commit
that referenced
this pull request
Sep 6, 2023
* Added overwrite parameter to catalog client.Put Signed-off-by: Nick Müller <[email protected]> * Refactored catalog client artifact overwrite into separate Update method Signed-off-by: Nick Müller <[email protected]> * Updated flyteidl and flytestdlib to latest released versions Signed-off-by: Nick Müller <[email protected]> Signed-off-by: Nick Müller <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
TL;DR
The catalog client's
Put
now has an additional parameteroverwrite
, indicating whether the artifact should be created (false
) or updated (true
).Type
Are all requirements met?
Complete description
The catalog client interface's
Put
was previously only used to create artifacts. In order to support the cache eviction RFC (and its first step of overwriting existing results for a single execution), anoverwrite
parameter was added, indicating the data store should replace an existing artifact instead of creating a new one.As the current implementation is already called
Put
and the HTTP verbPUT
is expected to replace existing data as well, I've opted for extending the current signature instead of creating a separate new method.Tracking Issue
flyteorg/flyte#2867
Follow-up issue
NA