feat: handle new export in CLI #11803

betodealmeida · 2020-11-24T23:24:25Z

SUMMARY

This PR adds the new export functionality to the superset CLI, when the VERSIONED_EXPORT feature flag is enabled.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

Tested with:

$ superset export-dashboards
$ superset export-datasources

Verified that files were correct.

ADDITIONAL INFORMATION

codecov-io · 2020-11-25T00:47:59Z

Codecov Report

Merging #11803 (4911039) into master (14ea444) will decrease coverage by 13.10%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #11803       +/-   ##
===========================================
- Coverage   66.22%   53.11%   -13.11%     
===========================================
  Files         937      438      -499     
  Lines       45325    15714    -29611     
  Branches     4341     4060      -281     
===========================================
- Hits        30016     8347    -21669     
+ Misses      15177     7367     -7810     
+ Partials      132        0      -132

Flag	Coverage Δ
cypress	`53.11% <ø> (+8.75%)`	⬆️
javascript	`?`
python	`?`

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

Impacted Files	Coverage Δ
...uperset-frontend/src/dashboard/util/dnd-reorder.js	`0.00% <0.00%> (-100.00%)`	⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js	`0.00% <0.00%> (-100.00%)`	⬇️
...dashboard/components/resizable/ResizableHandle.jsx	`0.00% <0.00%> (-100.00%)`	⬇️
.../src/dashboard/util/getFilterScopeFromNodesTree.js	`0.00% <0.00%> (-93.48%)`	⬇️
...src/dashboard/components/gridComponents/Header.jsx	`10.52% <0.00%> (-86.85%)`	⬇️
...rc/dashboard/components/gridComponents/Divider.jsx	`13.33% <0.00%> (-86.67%)`	⬇️
...ontend/src/dashboard/util/getDashboardFilterKey.ts	`14.28% <0.00%> (-85.72%)`	⬇️
...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx	`14.70% <0.00%> (-82.97%)`	⬇️
...set-frontend/src/views/CRUD/welcome/EmptyState.tsx	`5.71% <0.00%> (-82.10%)`	⬇️
...et-frontend/src/SqlLab/components/TableElement.jsx	`4.70% <0.00%> (-81.35%)`	⬇️
... and 833 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 14ea444...4911039. Read the comment docs.

john-bodley · 2020-11-25T03:31:13Z

superset/cli.py

+
+        g.user = security_manager.find_user(username="admin")
+
+        dashboard_ids = [id_ for (id_,) in db.session.query(Dashboard.id).all()]


You can drop the () if you want.

I think without the () it's easy to overlook what's happening:

dashboard_ids = [id_ for id_, in db.session.query(Dashboard.id).all()]

Adding parentheses make the intent more clear, IMHO.

[dashboard.id for dashboard in db.session.query(Dashboard.id).all()]

is clearer, but no strong opinions about it

@dpgaspar do you mean?

[dashboard.id for dashboard in db.session.query(Dashboard).all()]

That's a more expensive query (SELECT * FROM dbs instead of SELECT id FROM dbs), no?

yes, but no, I mean the way I wrote it that would generate a SELECT id from dbs but it's just a detail, if you feel more comfortable unpacking the tuples the way you wrote got ahead.

john-bodley · 2020-11-25T03:37:03Z

superset/cli.py

+        dashboard_ids = [id_ for (id_,) in db.session.query(Dashboard.id).all()]
+        timestamp = datetime.now().strftime("%Y%m%dT%H%M%S")
+        root = f"dashboard_export_{timestamp}"
+        dashboard_file = dashboard_file or f"{root}.zip"


Should the default (fallback) be expressed as a click option?

I played with click trying to have options depend on the feature flag, but couldn't get it to work since it depends on the app context. Let me give it another try.

dpgaspar

Looks good, just a couple of non blocking comments

superset/cli.py

ktmud · 2020-12-16T03:45:39Z

superset/cli.py

+feature_flags_func = config.GET_FEATURE_FLAGS_FUNC
+if feature_flags_func:
+    # pylint: disable=not-callable
+    feature_flags = feature_flags_func(feature_flags)


This breaks the CLI when feature_flags_func uses g.user or any other app context based variables, which seems to be the original intention of GET_FEATURE_FLAGS_FUNC.

RuntimeError: Working outside of application context.

cc @betodealmeida @dpgaspar

I added a quick workaround in #12088.

This reverts commit 862c251.

pull-request-size bot added the size/L label Nov 24, 2020

betodealmeida force-pushed the SO-1151 branch from 9f35df8 to 49b0598 Compare November 24, 2020 23:34

pull-request-size bot added size/M and removed size/L labels Nov 25, 2020

betodealmeida requested a review from john-bodley November 25, 2020 00:15

betodealmeida marked this pull request as ready for review November 25, 2020 00:15

betodealmeida requested review from hughhhh and removed request for john-bodley November 25, 2020 00:15

john-bodley reviewed Nov 25, 2020

View reviewed changes

betodealmeida force-pushed the SO-1151 branch from 49b2c3a to f93056e Compare November 25, 2020 06:54

pull-request-size bot added size/L and removed size/M labels Dec 4, 2020

betodealmeida added 4 commits December 5, 2020 16:20

feat: handle new export in CLI

011471a

Fix imports

30762e1

Fix lint

3ca7156

Set options based on feature flag

ca172f7

betodealmeida force-pushed the SO-1151 branch from 7a9a056 to ca172f7 Compare December 6, 2020 00:24

betodealmeida added 2 commits December 6, 2020 11:40

Fix lint

0a0d446

Fix lint

88d9c9d

betodealmeida requested review from john-bodley and dpgaspar December 15, 2020 03:07

dpgaspar approved these changes Dec 15, 2020

View reviewed changes

superset/cli.py Outdated Show resolved Hide resolved

superset/cli.py Outdated Show resolved Hide resolved

superset/cli.py Outdated Show resolved Hide resolved

Add better error messages

4911039

betodealmeida merged commit 862c251 into apache:master Dec 15, 2020

ktmud reviewed Dec 16, 2020

View reviewed changes

john-bodley pushed a commit to airbnb/superset-fork that referenced this pull request Dec 17, 2020

Revert "feat: handle new export in CLI (apache#11803)"

164e822

This reverts commit 862c251.

betodealmeida mentioned this pull request Dec 17, 2020

fix: handle context-dependent feature flags in CLI #12088

Merged

6 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handle new export in CLI #11803

feat: handle new export in CLI #11803

betodealmeida commented Nov 24, 2020 •

edited

Loading

codecov-io commented Nov 25, 2020 •

edited

Loading

john-bodley Nov 25, 2020

betodealmeida Nov 25, 2020

dpgaspar Nov 27, 2020 •

edited

Loading

betodealmeida Nov 27, 2020

dpgaspar Dec 15, 2020

john-bodley Nov 25, 2020

betodealmeida Nov 25, 2020

dpgaspar left a comment

ktmud Dec 16, 2020 •

edited

Loading

betodealmeida Dec 17, 2020


		g.user = security_manager.find_user(username="admin")

		dashboard_ids = [id_ for (id_,) in db.session.query(Dashboard.id).all()]

feat: handle new export in CLI #11803

feat: handle new export in CLI #11803

Conversation

betodealmeida commented Nov 24, 2020 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov-io commented Nov 25, 2020 • edited Loading

Codecov Report

john-bodley Nov 25, 2020

Choose a reason for hiding this comment

betodealmeida Nov 25, 2020

Choose a reason for hiding this comment

dpgaspar Nov 27, 2020 • edited Loading

Choose a reason for hiding this comment

betodealmeida Nov 27, 2020

Choose a reason for hiding this comment

dpgaspar Dec 15, 2020

Choose a reason for hiding this comment

john-bodley Nov 25, 2020

Choose a reason for hiding this comment

betodealmeida Nov 25, 2020

Choose a reason for hiding this comment

dpgaspar left a comment

Choose a reason for hiding this comment

ktmud Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

betodealmeida Dec 17, 2020

Choose a reason for hiding this comment

betodealmeida commented Nov 24, 2020 •

edited

Loading

codecov-io commented Nov 25, 2020 •

edited

Loading

dpgaspar Nov 27, 2020 •

edited

Loading

ktmud Dec 16, 2020 •

edited

Loading