-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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 DASHBOARD_CACHE feature flag #26349
refactor: Removes the deprecated DASHBOARD_CACHE feature flag #26349
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26349 +/- ##
==========================================
+ Coverage 66.85% 66.88% +0.02%
==========================================
Files 1931 1938 +7
Lines 75355 75505 +150
Branches 8431 8467 +36
==========================================
+ Hits 50378 50499 +121
- Misses 22829 22847 +18
- Partials 2148 2159 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
UPDATING.md
Outdated
@@ -29,6 +29,8 @@ assists people when migrating to a new version. | |||
|
|||
### Breaking Changes | |||
|
|||
- [26349](https://github.com/apache/superset/issues/26349): Removes the deprecated `DASHBOARD_CACHE` feature flag. |
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.
What are the consequences of removing the cache? Is there merit in expanding why this was removed?
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.
I think this feature never worked correctly. It was Airbnb who introduced it and quickly disabled it because of problems reported by users while using the dashboards.
Getting an error trying to delete a dashboard in the UI and thought it may be related:
|
checking out the SHA right before this got merged in master fixes the issue @michael-s-molina @eschutho |
Thanks for reporting the issue @mistercrunch. I opened #26648 to fix it. |
SUMMARY
As part of the 4.0 approved initiatives, this PR removes the
DASHBOARD_CACHE
feature flag.The previous value of the feature flag 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