-
Notifications
You must be signed in to change notification settings - Fork 670
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] Datacatalog cache deletion #4655
base: master
Are you sure you want to change the base?
Conversation
Implemented new datacatalog functionality required for cache eviction Updated to latest unreleased version of flyteidl and flyteplugins Signed-off-by: Nick Müller <[email protected]>
Allows for fields to be explicity set/updated to nil Signed-off-by: Nick Müller <[email protected]>
Allows for re-use by cache manager Signed-off-by: Nick Müller <[email protected]>
Added endpoint for evicting execution cache Added endpoint for evicting task execution cache Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Extended reservation retrieval to allow querying via artifact tag in catalog client interface Signed-off-by: Nick Müller <[email protected]>
Added method to delete catalog artifact by ID Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
…epropeller and flytestdlib Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Added new CacheEvictionError message representing an error encountered during eviction of stored data Added new UpdateTaskExecution endpoint for updating task executions, currently only supporting cache eviction Signed-off-by: Nick Müller <[email protected]>
Ran go mod tidy Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
…oints Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
grpc-gateway parsing of URL params does not work for joined endpoint at the moment - fixed in major version upgrade Added extra CacheEvictionErrorCode enum entries Signed-off-by: Nick Müller <[email protected]>
…artifacts to datacatalog Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Nick Müller <[email protected]>
Implement deleting of artifacts as bulk operation Signed-off-by: Nick Müller <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4655 +/- ##
==========================================
+ Coverage 59.00% 59.07% +0.07%
==========================================
Files 645 647 +2
Lines 55578 55972 +394
==========================================
+ Hits 32792 33065 +273
- Misses 20194 20301 +107
- Partials 2592 2606 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…on-past-executions' into feature/datacatalog-cache-deletion
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
…che evicted Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
…ations Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
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.
lets cleanup imports and resolve merge conflicts and then merge!
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
Signed-off-by: Paul Dittamo <[email protected]>
put up change for console: flyteorg/flyteconsole#851 |
Tracking issue
#2867
Why are the changes needed?
Flyte doesn't support the deletion of cached task executions. Adding cache eviction/cleanup gives Flyte users more control over their data helping with things such as GDPR compliance + reducing their cache size.
Opted to support just single task executions as invalidating an entire workflow's cache can run into timeout issues going down the nesting of nodes.
What changes were proposed in this pull request?
How was this patch tested?
unit tests
Run workflow that caches task executions
Setup process
(NOTE - a follow up PR will be to get that change merged into console since console doesn't support CACHE_EVICT CatalogCacheStatus)
Screenshots
Check all the applicable boxes
Related PRs
Docs link