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

fix(native-filters): add support for versioned import/export #16017

Merged
merged 4 commits into from
Aug 2, 2021

Conversation

villebro
Copy link
Member

@villebro villebro commented Aug 2, 2021

SUMMARY

This PR fixes native filter metadata in versioned import/export of dashboards by doing the following:

  • export all datasets referenced in native filters, even those not referenced by charts in dashboard.
  • replaces datasetId with datasetUuid in native filter targets to ensure that dataset reference is based on UUID, not numeric id.

Other bycatch fixes that were found while testing:

  • Fix datatype of query_context in ImportV1ChartSchema, which should be String, not Dict (caused a marshmallow schema validation exception). Also make it accept None values and add JSON validation.
  • datasource_type in dataset_info was set as view if the table was a virtual table, which caused an exception when importing virtual datasets. Since virtual datasets are in fact regular table datasources, we just reference the datasource_type of the imported dataset.

SCREENSHOT

Now native filter targets are referencing datasetUuid instead of datasetId:
image

TESTING INSTRUCTIONS

  1. enable the "VERSIONED_EXPORT" feature flag
  2. export a dashboard that had only one chart and two native filters, one referencing the same dataset as the chart and another not referenced by the chart
  3. check that the native filter configurations in the dashboard metadata contain datasetUuid instead of datasetId references
  4. delete the exported dashboard, chart and dataset only referenced by the native filter
  5. import the exported bundle
  6. make sure the imported dashboard works correctly, and that both datasets are imported correctly and native filter configurations are correct

ADDITIONAL INFORMATION

query_context = fields.Dict()
query_context = fields.String()
Copy link
Member Author

Choose a reason for hiding this comment

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

This caused a schema validation exception and needed to be changed to the string type

Comment on lines -99 to +101
"datasource_type": "view" if dataset.is_sqllab_view else "table",
"datasource_type": dataset.datasource_type,
Copy link
Member Author

Choose a reason for hiding this comment

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

This also caused an import error, since virtual datasets are in fact regular tables (they just have the sql property set).

@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #16017 (6b0a652) into master (46188c1) will decrease coverage by 0.23%.
The diff coverage is 77.04%.

❗ Current head 6b0a652 differs from pull request most recent head 78d136e. Consider uploading reports for the commit 78d136e to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16017      +/-   ##
==========================================
- Coverage   77.03%   76.80%   -0.24%     
==========================================
  Files         989      990       +1     
  Lines       52413    52491      +78     
  Branches     6634     6639       +5     
==========================================
- Hits        40378    40317      -61     
- Misses      11812    11950     +138     
- Partials      223      224       +1     
Flag Coverage Δ
hive ?
mysql 81.57% <76.92%> (-0.06%) ⬇️
postgres 81.63% <76.92%> (-0.02%) ⬇️
presto ?
python 81.72% <78.84%> (-0.45%) ⬇️
sqlite 81.23% <78.84%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
superset/charts/commands/export.py 94.28% <ø> (ø)
superset/commands/importers/v1/examples.py 38.02% <0.00%> (ø)
superset/security/manager.py 91.90% <ø> (ø)
superset/dashboards/commands/export.py 83.09% <33.33%> (-7.23%) ⬇️
superset/dashboards/commands/importers/v1/utils.py 80.59% <41.17%> (-13.53%) ⬇️
...erset-frontend/src/datasource/DatasourceEditor.jsx 74.15% <66.66%> (-0.85%) ⬇️
superset/connectors/sqla/utils.py 91.66% <91.66%> (ø)
superset/views/datasource.py 90.12% <95.00%> (+1.59%) ⬆️
superset/charts/schemas.py 100.00% <100.00%> (ø)
superset/connectors/sqla/models.py 88.00% <100.00%> (-1.84%) ⬇️
... and 11 more

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 46188c1...78d136e. Read the comment docs.

@zhaoyongjie zhaoyongjie self-requested a review August 2, 2021 11:47
Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

Tested that all the new created dashboard are working fine.(both importing and exporting)

@villebro villebro merged commit c77bf26 into apache:master Aug 2, 2021
@villebro villebro deleted the villebro/import-export-v2 branch August 2, 2021 13:11
@junlincc junlincc added dashboard:native-filters Related to the native filters of the Dashboard #bug:blocking! Blocking issues with high priority labels Aug 2, 2021
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
…16017)

* fix(native-filters): add support for versioned import/export

* fix lint

* address CR

* make query_context nullable
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
…16017)

* fix(native-filters): add support for versioned import/export

* fix lint

* address CR

* make query_context nullable
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
…16017)

* fix(native-filters): add support for versioned import/export

* fix lint

* address CR

* make query_context nullable
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
…16017)

* fix(native-filters): add support for versioned import/export

* fix lint

* address CR

* make query_context nullable
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.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 #bug:blocking! Blocking issues with high priority dashboard:native-filters Related to the native filters of the Dashboard preset-io size/M v1.3 🚢 1.3.0
Projects
None yet
5 participants