-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
refactor: Removes the deprecated KV Store feature #26376
refactor: Removes the deprecated KV Store feature #26376
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26376 +/- ##
==========================================
- Coverage 69.18% 69.13% -0.05%
==========================================
Files 1946 1946
Lines 75988 75943 -45
Branches 8479 8478 -1
==========================================
- Hits 52570 52502 -68
- Misses 21228 21258 +30
+ Partials 2190 2183 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Good riddance! LGTM
@villebro I think this might be more nuanced than simply removing feature flags as there's no clear migration plan away from previous shared SQL Lab links which users likely perceive as somewhat persistent in nature. We (Airbnb) are discussing how best to proceed with preserving these semi-permanent URLs for a finite period. That said, I'll be stoked when we finally remove this logic for good. |
@john-bodley One idea that came to mind - maybe you could just introduce a new namespace into the new kv store, migrate all existing legacy links to it and add the (now to be removed) endpoint to your local app extension that serves from the new kv store? If being able to serve these ancient links post 4.0 is important that is.. |
@michael-s-molina note that both the I'm not certain (per the Semantic Versioning wiki page) whether i) these feature flags need to be first marked as deprecated and/or if there removal is deemed a breaking change given that (per the wiki):
|
Closing in favor of #26450 |
SUMMARY
As part of the 4.0 approved initiatives, this PR removes the deprecated KV Store feature and its related assets such as the API endpoint, the
keyvalue
table, and theKV_STORE
andSHARE_QUERIES_VIA_KV_STORE
feature flags.The previous value of both feature flags was
False
and now the feature is permanently removed.TESTING INSTRUCTIONS
CI should be sufficient for merging this PR. We'll do a complete testing of 4.0 after all approved proposals are merged.
ADDITIONAL INFORMATION