Skip to content
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

[perf_logging] Add is_cached status when chart has error #9009

Merged

Conversation

graceguo-supercat
Copy link

@graceguo-supercat graceguo-supercat commented Jan 24, 2020

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

when user loads a dashboard, if query returns 0 row, we will show no data and response 400. Superset front-end treat it as error. But the result will still be cached.
In front-end logging, we don't log is_cached status when chart has error. this may cause our cache rate not accurate. This PR is trying to fix this case (chart has error, and may also has is_cached status).

TEST PLAN

manual test

REVIEWERS

@etr2460

Copy link
Member

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, thanks for the fix!

@codecov-io
Copy link

Codecov Report

Merging #9009 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9009   +/-   ##
=======================================
  Coverage   59.15%   59.15%           
=======================================
  Files         367      367           
  Lines       11682    11682           
  Branches     2864     2864           
=======================================
  Hits         6910     6910           
  Misses       4593     4593           
  Partials      179      179
Impacted Files Coverage Δ
superset/assets/src/chart/chartAction.js 47.94% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3d8603...690f7cb. Read the comment docs.

@graceguo-supercat graceguo-supercat merged commit d0bc04f into apache:master Jan 24, 2020
@graceguo-supercat graceguo-supercat deleted the gg-FixIsCachedOnError branch February 10, 2020 04:18
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 0.36.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants