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

feat: configure force_screenshot #17855

Merged
merged 11 commits into from
Dec 22, 2021
Merged

feat: configure force_screenshot #17855

merged 11 commits into from
Dec 22, 2021

Conversation

betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented Dec 22, 2021

SUMMARY

Allow users to force a screenshot (bypass the cache) on reports that send charts.

We're not enabling this for dashboards yet, since it still needs backend support.

For alerts, this is always on for charts, and off for dashboards (for perf reasons, and also because it's not implemented).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screenshot 2021-12-12 at 12-46-09  DEV  Superset

TESTING INSTRUCTIONS

  1. Create a report, toggle the new checkbox.
  2. Check that force_screenshot is updated accordingly in the DB, or run celery and check that screenshot URLs have force=true or force=false.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@github-actions
Copy link
Contributor

⚠️ @betodealmeida Your base branch ch24621b has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@codecov
Copy link

codecov bot commented Dec 22, 2021

Codecov Report

Merging #17855 (5c2a680) into master (2cd8054) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #17855   +/-   ##
=======================================
  Coverage   66.91%   66.91%           
=======================================
  Files        1609     1609           
  Lines       64887    64887           
  Branches     6863     6863           
=======================================
  Hits        43419    43419           
  Misses      19603    19603           
  Partials     1865     1865           
Flag Coverage Δ
mysql 82.19% <ø> (ø)
postgres 82.24% <ø> (ø)
python 82.32% <ø> (ø)
sqlite 81.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset-frontend/src/views/CRUD/alert/types.ts 100.00% <ø> (ø)
superset/reports/api.py 88.46% <ø> (ø)
superset/reports/commands/execute.py 91.28% <ø> (ø)
...frontend/src/views/CRUD/alert/AlertReportModal.tsx 53.54% <50.00%> (ø)

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 2cd8054...5c2a680. Read the comment docs.

@betodealmeida betodealmeida changed the base branch from ch24621b to master December 22, 2021 20:16
@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 22, 2021
@github-actions
Copy link
Contributor

⚠️ @betodealmeida Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@betodealmeida betodealmeida merged commit 9baeafe into master Dec 22, 2021
shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
* Update existing tests

* Add backend test

* feat: add force option to report screenshots

* Add tests

* Rebase fixes

* Do not force screenshot on dashboard alerts

* feat: bypass cache on screenshots for alerts

* Update existing tests

* Add tests

* feat: configure force_screenshot
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
* Update existing tests

* Add backend test

* feat: add force option to report screenshots

* Add tests

* Rebase fixes

* Do not force screenshot on dashboard alerts

* feat: bypass cache on screenshots for alerts

* Update existing tests

* Add tests

* feat: configure force_screenshot
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
@mistercrunch mistercrunch deleted the ch24621c branch March 26, 2024 16:08
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/M 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants