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: Allows dynamic type on viz migrations #23975

Conversation

michael-s-molina
Copy link
Member

SUMMARY

This PR modifies the viz migration base class to allow the target viz type to be defined dynamically during the migration itself. This is a prerequisite for some types of migrations such as the NVD3 Line chart which can be migrated into different ECharts charts such as Line, Smooth Line, Stepped Line, etc.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • 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

@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #23975 (f1a7b48) into master (b4371f6) will increase coverage by 0.02%.
The diff coverage is 94.46%.

❗ Current head f1a7b48 differs from pull request most recent head 5e9fe8d. Consider uploading reports for the commit 5e9fe8d to get more accurate results

@@            Coverage Diff             @@
##           master   #23975      +/-   ##
==========================================
+ Coverage   68.15%   68.18%   +0.02%     
==========================================
  Files        1941     1941              
  Lines       75212    75213       +1     
  Branches     8158     8153       -5     
==========================================
+ Hits        51262    51284      +22     
+ Misses      21867    21848      -19     
+ Partials     2083     2081       -2     
Flag Coverage Δ
hive 53.17% <96.48%> (-0.01%) ⬇️
mysql 78.90% <98.99%> (?)
postgres 78.97% <98.99%> (-0.01%) ⬇️
presto 53.09% <96.48%> (-0.01%) ⬇️
python 82.75% <98.99%> (+0.04%) ⬆️
sqlite 77.49% <98.99%> (-0.01%) ⬇️
unit 52.98% <96.48%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
superset-frontend/src/SqlLab/App.jsx 0.00% <ø> (ø)
...frontend/src/components/DatabaseSelector/index.tsx 91.07% <ø> (-0.46%) ⬇️
...et-frontend/src/components/TableSelector/index.tsx 78.37% <ø> (ø)
...uperset-frontend/src/hooks/apiResources/schemas.ts 50.00% <ø> (-42.31%) ⬇️
superset-frontend/src/hooks/apiResources/tables.ts 80.00% <ø> (-4.22%) ⬇️
superset-frontend/src/views/App.tsx 0.00% <ø> (ø)
superset-frontend/src/views/store.ts 78.26% <ø> (ø)
superset/datasource/api.py 100.00% <ø> (ø)
superset/db_engine_specs/ocient.py 57.28% <0.00%> (ø)
...d/src/SqlLab/components/TabbedSqlEditors/index.jsx 50.00% <50.00%> (+0.52%) ⬆️
... and 48 more

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Thanks for clarifying things on Slack about the test coverage (happening via the individual migration PRs), and the clarity on granularity_sqla (terrible name / refactor ahead). This otherwise makes sense to me... let's do it!

@michael-s-molina michael-s-molina merged commit 7fb8b38 into apache:master May 10, 2023
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 13, 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/S 🚢 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants