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: add force option to report screenshots #39

Closed
wants to merge 77 commits into from
Closed

Conversation

betodealmeida
Copy link
Owner

@betodealmeida betodealmeida commented Dec 11, 2021

SUMMARY

Add a new column force_screenshot to the reports/alerts model. For alerts this is always set to true. For reports this is currently set to false, and will be made configurable.

When the flag is true, when generating a screenshot for the alert/report the cache will be bypassed.

Depends on apache#17695.

MIGRATION

DB migration adds a new column force_screenshot, set to true for existing alerts, false otherwise.

Results:

Current: 0.34 s
10+: 0.35 s
100+: 0.39 s
1000+: 0.74 s
10000+: 3.58 s
100000+: 32.59 s
1000000+: 453.84 s

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  1. Create a report and a alert.
  2. Run SELECT force_screenshot FROM report_schedule, check that it's true for the alert and false for the report.

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

ofekisr and others added 18 commits December 8, 2021 20:43
* chore: removing redundant docker-entrypoint

* chore: chmod the run-server

* Update Dockerfile

Co-authored-by: Amit Miran <[email protected]>
* Redirect on 401

* Bump FAB

* Format

* Update Cypress save test

* Revert Cypress change

* Bump FAB 3.4.1rc2

* Update test

* Update return statement

* Update api test

* Update datasets api test

* Update datasets api 401s to 403s

* Add typeguard

* Use Promise.resolve

* Update callApiAndParseWithhTimeout test

* Disable parseResponse test

* Try catch

* Handle npm 8 issues
* chore: convert feature flag tests to unit tests

* Fix test
)

* chore(sql): clean up invalid filter clause exception types

* fix lint

* rename exception
…pache#17638)

* fix: Select component sort function sorting by label instead of value on numbers

* fix: change select component default sorting to sort by the initial index rather than a property like value or label

* fix: select sorting add sortOptions to select components using sortByProperty

* fix: change select component back, add order to options coming in from SelectControl

* fix: select component options intitial sort bug

* fix: add test cases for select fix

Co-authored-by: Corbin Robb <[email protected]>
* Refactor PropertiesModal

* Update json_metadata fully

* Clean up

* Verify values

* Catch changed to metadata

* Always updated dashboard info on update

* Avoid unnecessary fetches

* Formt

* Fix copy dashboards

* Fixes onUpdate onCopy handlers

* Pylint

* Update tests

* Clean up

* Handle data on show

* Change Save to Apply

* Update Cypress save test

* Update Cypress edit prop test

* Update PropertiesModal test

* Fix duplicate request with cross filters

* Improve code style

* Fix typo

* Lint
* fix(postgres): remove redundant tz factory

* lint
@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2021

Codecov Report

Merging #39 (ac44dd0) into ch24621 (ff741e9) will increase coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@           Coverage Diff            @@
##           ch24621      #39   +/-   ##
========================================
  Coverage    68.88%   68.88%           
========================================
  Files         1598     1598           
  Lines        65297    65301    +4     
  Branches      6952     6953    +1     
========================================
+ Hits         44978    44981    +3     
- Misses       18434    18435    +1     
  Partials      1885     1885           
Flag Coverage Δ
hive 81.72% <100.00%> (+<0.01%) ⬆️
javascript 57.47% <0.00%> (-0.01%) ⬇️
mysql 82.14% <100.00%> (+<0.01%) ⬆️
postgres 82.15% <100.00%> (+<0.01%) ⬆️
python 82.49% <100.00%> (+<0.01%) ⬆️
sqlite 81.83% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...rset-frontend/src/components/ReportModal/index.tsx 82.55% <ø> (ø)
...frontend/src/views/CRUD/alert/AlertReportModal.tsx 53.28% <0.00%> (-0.15%) ⬇️
superset/db_engine_specs/postgres.py 97.36% <100.00%> (ø)
superset/models/reports.py 100.00% <100.00%> (ø)
superset/reports/commands/execute.py 91.28% <100.00%> (ø)
superset/reports/schemas.py 98.80% <100.00%> (+0.02%) ⬆️

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 ff741e9...ac44dd0. Read the comment docs.

@betodealmeida betodealmeida force-pushed the ch24621b branch 2 times, most recently from f874d2f to ac44dd0 Compare December 12, 2021 19:39
dpgaspar and others added 9 commits December 13, 2021 12:03
* fix: migration out-of-scope bind

* Bypass flaky test
…atype names for complexed columns (apache#17360)

* Change datatype of column type in BaseColumn to allow larger datatype names for complexed columns

* Fixed formatting

* Added an entry in the UPDATING.md file as a potential downtime

* Update UPDATING.md

Accept proposed changes.

Co-authored-by: Beto Dealmeida <[email protected]>

* Updated down revision number to reflect new head

Co-authored-by: cccs-joel <[email protected]>
Co-authored-by: Beto Dealmeida <[email protected]>
* fix(Mixed Timeseries Chart): Custom Metric Label

* Fixed Formatting

* Fixed Type mismatch from queryFormData

* Reverted type change and used extracted datasource

* Type fix for mapping
* chore: bump FAB to 3.4.1

* test
ChristopherCFleming and others added 26 commits December 16, 2021 21:51
* Line 976 was wakeup, now wake up

* Removed package lock to test environment

* Return original package-lock.json
* fix: [alert] add tooltip message for alert threshold value

* support decimal value for alert condition threshold

* add integration test
…et codebase (apache#17769)

* chore(plugin-chart-pivot-table): migrate react-pivottable into superset codebase

* Fix lint errors

* Use named export

* Clean up the code
…he#17650)

* chore: updated screenshots in readme.md and added recent events

* chore: updated pinot superset connection instructions

* fixed outdated slack link

* addressed all outstanding feedback!

* updated package lock

* reverted package.json to master

* Update README.md

Co-authored-by: Evan Rusackas <[email protected]>

Co-authored-by: Evan Rusackas <[email protected]>
…17811)

* Add option to provide configOverrides through file

* Bump chart to 0.5.1
* chore(generator-superset): migrate to monorepo

* add todo and remove webpack reference from template

* fix linting errors

* remove redundant test file
* feat(plugin-chart-pivot-table): support series limit

* Add a migration

* Use non-legacy series limit controls

* Add a todo comment

* Bug fix
* afirst stage to ccheck to get initial datamask

* clean up code and update typescript

* remove consoles

* fix ts and update copy dashboard url

* use key when one doesn't exists

* lint clean up

* fix errors

* add suggested changes

* remove line

* add tests and add changes for copydashboard

* fix lint

* fix lint

* fix lint

* Update superset-frontend/src/dashboard/components/Header/index.jsx

Co-authored-by: Ville Brofeldt <[email protected]>

* add timeout

* fix test

* fix test, add qs to cypress and add suggestions

* add suggestions

* fix lint

* more suggested changes for backwards comapat

* fix lint

* cleanup naming and add qs parse to tests

* Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx

Co-authored-by: Ville Brofeldt <[email protected]>

* Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx

Co-authored-by: Ville Brofeldt <[email protected]>

* more changes and fix lint

* remove nativefiler param

* fix path

* remove con

* simplify logic

Co-authored-by: Ville Brofeldt <[email protected]>
* refactor(monorepo): use local dependencies in packages.json

* add prettier ignore

* update commit message

* fix release workflow

* refine release script
* feat: bypass cache on screenshots for alerts

* Update existing tests

* Add backend test

* Add frontend test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.