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

chore: improve export by ID #88

Merged
merged 2 commits into from
Sep 28, 2022
Merged

chore: improve export by ID #88

merged 2 commits into from
Sep 28, 2022

Conversation

betodealmeida
Copy link
Member

This PR introduces a few changes to the superset export command. First, if IDs are specified for any resource type, then all assets are exported by ID. The command:

superset-cli export --database-ids=1,2

Will export only databases with ID 1 and 2, and no datasets, charts, nor dashboards will be exported. Before: this would export databases with ID 1 and 2, but also all datasets, charts, and dashboards.

Second, when an ID is specified, related assets are also exported. The command:

superset-cli export --dashboard-ids=1 --asset-type=dashboard

Will export dashboard with ID 1, but also all its associated charts, datasets, and databases. Before: this would export only the dashboard with ID 1, but not export associated resources.

IDs for multiple resource types can be specified:

superset-cli export --dashboard-ids=1 --chart-ids=42

Will export dashboard with ID 1 and related assets, as well as chart with ID 42 and related assets.

@@ -77,18 +77,29 @@ def export_assets( # pylint: disable=too-many-locals, too-many-arguments
"chart": {int(id_) for id_ in chart_ids},
"dashboard": {int(id_) for id_ in dashboard_ids},
}
ids_requested = database_ids or dataset_ids or chart_ids or dashboard_ids
Copy link
Member

@mistercrunch mistercrunch Sep 28, 2022

Choose a reason for hiding this comment

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

NIT(optional): could be a tiny bit more clear is

were_any_ids_requested = any([database_ids, dataset_ids, chart_ids, dashboard_ids])

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I like it! Let me fix.

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

Minor optional comment, otherwise LGTM!

@betodealmeida betodealmeida merged commit 949e2a7 into main Sep 28, 2022
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.

None yet

2 participants