Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: dataset extra import/export #17740

Merged
merged 2 commits into from
Dec 22, 2021
Merged

Conversation

betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented Dec 14, 2021

SUMMARY

The extra attribute from datasets is being imported as a string. The resulting YAML looks like this:

extra: '{{\"warning_markdown\": \"*WARNING*\"}}'

When it should look like this:

extra:
  warning_markdown: '*WARNING*'

I fixed the export to run json.load on the field before the export, and fixed the import to support both formats.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

I added unit tests covering the export, and imports for the old and new format.

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

@betodealmeida betodealmeida changed the title Ch31665b fix: dataset extra import/export Dec 15, 2021
@betodealmeida betodealmeida force-pushed the ch31665b branch 2 times, most recently from 603a70e to 5d52ab8 Compare December 15, 2021 15:41
@betodealmeida betodealmeida marked this pull request as ready for review December 15, 2021 15:53
@betodealmeida betodealmeida force-pushed the ch31665b branch 2 times, most recently from 31e3cfa to 8cf9c7c Compare December 15, 2021 18:53
@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #17740 (22c80a9) into master (82b47ca) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17740      +/-   ##
==========================================
- Coverage   67.71%   67.61%   -0.11%     
==========================================
  Files        1605     1605              
  Lines       64198    64218      +20     
  Branches     6790     6790              
==========================================
- Hits        43474    43418      -56     
- Misses      18868    18944      +76     
  Partials     1856     1856              
Flag Coverage Δ
hive ?
mysql 82.18% <100.00%> (+0.01%) ⬆️
postgres 82.24% <100.00%> (+0.01%) ⬆️
python 82.32% <100.00%> (-0.25%) ⬇️
sqlite 81.92% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
superset/datasets/commands/export.py 87.50% <100.00%> (ø)
superset/datasets/commands/importers/v1/utils.py 58.33% <100.00%> (ø)
superset/datasets/schemas.py 96.92% <100.00%> (+0.31%) ⬆️
superset/db_engines/hive.py 0.00% <0.00%> (-85.19%) ⬇️
superset/db_engine_specs/hive.py 70.27% <0.00%> (-16.99%) ⬇️
superset/views/database/mixins.py 81.03% <0.00%> (-1.73%) ⬇️
superset/utils/webdriver.py 78.65% <0.00%> (-1.14%) ⬇️
superset/db_engine_specs/presto.py 83.50% <0.00%> (-0.84%) ⬇️
superset/dashboards/dao.py 95.55% <0.00%> (-0.75%) ⬇️
superset/db_engine_specs/base.py 88.29% <0.00%> (-0.39%) ⬇️
... and 12 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 82b47ca...22c80a9. Read the comment docs.

@betodealmeida betodealmeida mentioned this pull request Dec 16, 2021
9 tasks
Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

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

I just left a nit question

@betodealmeida betodealmeida added the need:merge The PR is ready to be merged label Dec 22, 2021
@betodealmeida betodealmeida merged commit c49545a into apache:master Dec 22, 2021
shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
* fix: dataset extra import/export

* Update superset/datasets/commands/importers/v1/utils.py
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
* fix: dataset extra import/export

* Update superset/datasets/commands/importers/v1/utils.py
@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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels lts-v1 need:merge The PR is ready to be merged size/M 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants