fix(revert): "fix: cache warmup solution non legacy charts. (#23012)"#23579
Conversation
This reverts commit e755b4f.
Codecov Report
@@ Coverage Diff @@
## master #23579 +/- ##
=======================================
Coverage 67.71% 67.72%
=======================================
Files 1916 1916
Lines 74014 74007 -7
Branches 8039 8039
=======================================
- Hits 50122 50118 -4
+ Misses 21843 21840 -3
Partials 2049 2049
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
villebro
left a comment
There was a problem hiding this comment.
LGTM - I created a filter box, and lo and behold, it indeed has a query context, and based on my quick inspection the payload is invalid (=would return a 400 if sent to the chart data endpoint).
After this revert is merged I can work on a new PR so that
- saving a legacy chart sets
query_contexttoNULLand fixes existing chart metadata (db migration) - reintroduces the functionality from the reverted PR
|
Ok thanks for the heads up @john-bodley . I'll make sure to incorporate the work from all prior PRs in my fix 👍 |
…3012)" (apache#23579) (cherry picked from commit b58d17f)
SUMMARY
This reverts #23012 which per #23012 (comment) introduced two known regressions. Granted we could try to roll forward with a formulation which works for both legacy and non-legacy charts, but in the interim I think it's prudent we ensure we don't regress whilst acknowledging the fact that cache warmup for non-legacy charts is still problematic.
cc: @dheeraj-jaiswal-lowes
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION