feat: create backend routes and API for importing saved queries #13893

AAfghahi · 2021-03-31T18:42:21Z

SUMMARY

Created backend routes and validation for Import Saved Queries feature that Superset is rolling out.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

none yet

TEST PLAN

none yet!

ADDITIONAL INFORMATION

Has associated issue:
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

superset/queries/saved_queries/api.py

betodealmeida · 2021-03-31T18:51:23Z

superset/queries/saved_queries/api.py

+        log_to_statsd=False,
+    )
+    def import_(self) -> Response:
+        """Import chart(s) with associated datasets and databases


chart -> saved_query (same thing in other places)

embarrassing, thank you for pointing it out.

I've done that so many times! 😆

betodealmeida · 2021-03-31T18:58:56Z

superset/queries/saved_queries/api.py

+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+        log_to_statsd=False,
+    )
+    def post(self) -> Response:


We don't need this for this PR.

Creating a new saved query would be an API for when the user clicks "save query" in SQL Lab. Currently that flow uses the old API (savedqueryviewapi/api/create), but when we eventually migrate it to the new v1 API this is the endpoint that would be used.

I would suggest moving this out of the PR, and have a separate PR introducing this and the command, so we can move "save query" to this endpoint.

betodealmeida · 2021-03-31T18:59:12Z

superset/queries/saved_queries/commands/create.py

@@ -0,0 +1,56 @@
+# Licensed to the Apache Software Foundation (ASF) under one


This is also not needed, let's move to a separate PR.

betodealmeida · 2021-03-31T19:01:15Z

superset/queries/saved_queries/commands/importers/v1/_init_.py

+    prefix ="saved_queries/"
+    schemas: Dict[str, Schema] = {
+        "datasets/": ImportV1DatasetSchema(),
+        "saved_query": ImportV1SavedQuerySchema(),


queries/, this maps to the directory inside the ZIP file

by this do you mean changing it to saved_queries or replacing it with queries/ altogether. Furthermore, would I then use ImportV1SavedQuerySchema still?

This is used when validating the schema of all the files inside the ZIP: https://github.com/apache/superset/blob/master/superset/commands/importers/v1/__init__.py#L108-L111

If you open the ZIP file from a saved query export you'll see that saved queries are stored inside the queries/ subdirectory, so we need to use that:

schemas: Dict[str, Schema] = { "datasets/": ImportV1DatasetSchema(), "queries/": ImportV1SavedQuerySchema(),

ok that makes a lot of sense. Thank you.

betodealmeida · 2021-03-31T19:02:59Z

superset/queries/saved_queries/commands/importers/v1/_init_.py

+        # discover datasets associated with saved queries
+        dataset_uuids: Set[str] = set()
+        for file_name, config in configs.items():
+            if file_name.startswith("saved_queries/"):


Same here, currently the directory is queries/. (Check the ZIP file with an exported saved query.)

(We can't change it to saved_queries/ unless we bump the version of the importer.)

betodealmeida · 2021-03-31T19:04:46Z

superset/queries/saved_queries/commands/importers/v1/_init_.py

+    def _import(
+        session: Session, configs: Dict[str, Any], overwrite: bool = False
+    ) -> None:
+        # discover datasets associated with saved queries


Saved queries are not associated with datasets, they're associated with databases only. This is how the exported saved query looks like:

schema: master label: Untitled Query 2 description: test sql: SELECT * FROM covid_dashboard WHERE WHERE FROM HA AVG SUM MAX uuid: 5a8be71f-f59f-4a7c-a7b3-e59394168062 version: 1.0.0 database_uuid: 45812a92-6844-4578-8692-0cc603c3fed0

So you need to scan saved queries, find the associated databases, import them, then import the saved queries. There's no datasets involved here.

def _import( session: Session, configs: Dict[str, Any], overwrite: bool = False ) -> None: # discover databases associated with saved queries dataset_uuids: Set[str] = set() for file_name, config in configs.items(): if file_name.startswith("queries/"): dataset_uuids.add(config["database_uuid"]) # import related databases database_ids: Dict[str, int] = {} for file_name, config in configs.items(): if file_name.startswith("databases/") and config["uuid"] in database_uuids: database = import_database(session, config, overwrite=False) database_ids[str(database.uuid)] = database.id # import saved queries with the correct parent ref for file_name, config in configs.items(): if file_name.startswith("queries/") and config["database_uuid"] in database: # update datasource id, type, and name database = database[config["dataset_uuid"]] config.update( { "datasource_id": database.id, "datasource_name": database.table_name, } ) config["params"].update({"datasource": database.uuid}) import_saved_query(session, config, overwrite=overwrite)

is what I set it to, if I am understanding this correctly. Though I'd like to talk through what this file is doing

betodealmeida · 2021-03-31T19:06:45Z

Don't forget to add unit tests, you can find examples for all the other imports inside tests.

betodealmeida · 2021-03-31T23:50:36Z

superset/queries/saved_queries/commands/exceptions.py

@@ -25,3 +30,10 @@ class SavedQueryBulkDeleteFailedError(DeleteFailedError):

 class SavedQueryNotFoundError(CommandException):
    message = _("Saved query not found.")
+
+class SavedQueryImportError(ImportFailedError):
+    message = _("Import chart failed for an unknown reason")


End the message in a period for consistency.

betodealmeida · 2021-03-31T23:50:55Z

superset/queries/saved_queries/commands/exceptions.py

+    message = _("Import chart failed for an unknown reason")
+
+class SavedQueryInvalidError(CommandInvalidError):
+    message = _("Saved Query Parameters are invalid")


message = _("Saved Query parameters are invalid.")

betodealmeida · 2021-03-31T23:52:13Z

superset/queries/saved_queries/commands/importers/v1/_init_.py

+
+    dao = SavedQueryDAO
+    model_name= "saved_queries"
+    prefix ="saved_queries/"


This also needs to be queries/, matching the name of the directory inside the ZIP file.

codecov · 2021-04-07T00:24:17Z

Codecov Report

Merging #13893 (a2235c3) into master (9d0bb3a) will increase coverage by 2.64%.
The diff coverage is 87.87%.

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

@@            Coverage Diff             @@
##           master   #13893      +/-   ##
==========================================
+ Coverage   76.78%   79.42%   +2.64%     
==========================================
  Files         935      939       +4     
  Lines       47292    47542     +250     
  Branches     5895     5938      +43     
==========================================
+ Hits        36311    37759    +1448     
+ Misses      10824     9657    -1167     
+ Partials      157      126      -31

Flag	Coverage Δ
cypress	`56.04% <74.07%> (+3.73%)`	⬆️
hive	`80.52% <83.76%> (+0.23%)`	⬆️
mysql	`80.82% <83.11%> (+0.21%)`	⬆️
postgres	`80.85% <83.11%> (+0.21%)`	⬆️
presto	`80.54% <85.06%> (+0.25%)`	⬆️
python	`81.40% <86.03%> (+0.26%)`	⬆️
sqlite	`80.41% <83.11%> (+0.22%)`	⬆️

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

Impacted Files	Coverage Δ
...tend/src/SqlLab/components/ScheduleQueryButton.tsx	`28.12% <ø> (ø)`
...et-frontend/src/SqlLab/components/TableElement.jsx	`88.60% <ø> (+0.23%)`	⬆️
...end/src/common/components/DropdownButton/index.tsx	`24.00% <0.00%> (ø)`
...frontend/src/components/DatabaseSelector/index.tsx	`92.70% <ø> (+1.04%)`	⬆️
...rset-frontend/src/components/ErrorMessage/types.ts	`100.00% <ø> (ø)`
superset-frontend/src/components/Icon/index.tsx	`100.00% <ø> (ø)`
...rset-frontend/src/components/ProgressBar/index.tsx	`100.00% <ø> (ø)`
...ontend/src/components/URLShortLinkButton/index.jsx	`100.00% <ø> (ø)`
.../components/CrossFilterScopingModal/utils/index.ts	`100.00% <ø> (ø)`
...ntend/src/dashboard/components/CssEditor/index.jsx	`95.83% <ø> (ø)`
... and 247 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 3c4591e...a48bd8c. Read the comment docs.

betodealmeida · 2021-04-07T00:40:01Z

superset/queries/saved_queries/commands/importers/v1/__init__.py

+
+        # import saved queries with the correct parent ref
+        for file_name, config in configs.items():
+            if file_name.startswith("queries/") and config["database_uuid"] in database:


database here is a Database object, you want:

Suggested change

if file_name.startswith("queries/") and config["database_uuid"] in database:

if file_name.startswith("queries/") and config["database_uuid"] in database_ids:

superset/queries/saved_queries/commands/importers/v1/__init__.py

betodealmeida · 2021-04-07T00:43:35Z

superset/queries/saved_queries/api.py

+        log_to_statsd=False,
+    )
+    def import_(self) -> Response:
+        """Import Saved Queries with associated datasets and databases


Suggested change

"""Import Saved Queries with associated datasets and databases

"""Import Saved Queries with associated databases

betodealmeida · 2021-04-07T00:43:49Z

superset/queries/saved_queries/api.py

+                      description: JSON map of passwords for each file
+                      type: string
+                    overwrite:
+                      description: overwrite existing databases?


Suggested change

description: overwrite existing databases?

description: overwrite existing saved queries?

betodealmeida · 2021-04-07T00:45:19Z

superset/queries/saved_queries/commands/importers/v1/__init__.py

+    """Import Saved Queries"""
+
+    dao = SavedQueryDAO
+    model_name= "saved_queries"


The lint is going to fail here and in a few other places. You can auto format it by running pre-commit run --all-files.

Suggested change

model_name= "saved_queries"

model_name = "saved query"

ok that saves me a lot of heart ache. ty

betodealmeida · 2021-04-07T00:54:27Z

tests/queries/saved_queries/api_tests.py

+        buf.seek(0)
+        return buf
+
+    @pytest.mark.usefixtures("create_saved_queries")


A fixture runs setup code before a test function, and cleanup code after. There's no fixture called create_saved_queries, so you should remove this.

that makes sense, sort of like a beforeEach afterEach in RTL. Would it be more thorough to add this fixture?

betodealmeida · 2021-04-07T00:55:56Z

tests/queries/saved_queries/commands_tests.py

+        assert saved_query.uuid == "05b679b5-8eaf-452c-b874-a7a774cfa4e9"
+        assert saved_query.database_uuid == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89"


I suspect these will change every time you run the tests, so you might need to remove these tests.

betodealmeida · 2021-04-07T00:56:49Z

tests/queries/saved_queries/commands_tests.py

+        assert saved_query.sql == (
+            """
+            -- Note: Unless you save your query, 
+            these tabs will NOT persist if you clear
+            your cookies or change browsers.
+
+            SELECT * from birth_names
+            """
+        )


This test will probably fail because the SQL is formatted differently (there's no line break after "your query,", eg).

Yeah, they do. Was wondering about this actually, should I just not test the sql or is there a comment to disable the line is too long linter?

You can disable to line it too linter, or break the line:

assert saved_query == ( "-- Note: Unless you save your query, " "these tabs will NOT persist if you clear " "our cookies or change browsers.\n\n" ... )

betodealmeida · 2021-04-07T00:57:16Z

tests/queries/saved_queries/commands_tests.py

+        db.session.commit()
+
+    def test_import_v1_saved_queries_validation(self):
+        """Test different validations applied when importing a chart"""


Suggested change

"""Test different validations applied when importing a chart"""

"""Test different validations applied when importing a saved query"""

betodealmeida · 2021-04-07T00:58:51Z

tests/queries/saved_queries/commands_tests.py

+        """Test different validations applied when importing a chart"""
+        # metadata.yaml must be present
+        contents = {
+            "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config),


You need to remove metadata.yaml, since here you're testing that a validation message "Missing metadata.yaml" is shown when the file is missing.

AAfghahi

Thank you

AAfghahi

ty

betodealmeida

AAfghahi · 2021-04-07T19:39:50Z

@betodealmeida Thank you! Though, its still failing two tests :/

superset/queries/saved_queries/commands/importers/v1/utils.py

Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>

…tils.py" This reverts commit 18580aa.

…he#13893) * initial commit * revisions * started tests * added unit tests * revisions * tests passing * fixed api test * Update superset/queries/saved_queries/commands/importers/v1/utils.py Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> * Revert "Update superset/queries/saved_queries/commands/importers/v1/utils.py" This reverts commit 18580aa. Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>

initial commit

a4a7bf9

pull-request-size bot added the size/L label Mar 31, 2021

betodealmeida self-requested a review March 31, 2021 18:47

betodealmeida reviewed Mar 31, 2021

View reviewed changes

superset/queries/saved_queries/api.py Show resolved Hide resolved

betodealmeida reviewed Mar 31, 2021

View reviewed changes

hughhhh self-requested a review March 31, 2021 20:46

betodealmeida reviewed Mar 31, 2021

View reviewed changes

AAfghahi force-pushed the saved_queries branch from 0dfd13e to b8ce2dd Compare April 1, 2021 15:17

AAfghahi changed the title ~~feature: create backend routes and API for importing saved queries~~ feat: create backend routes and API for importing saved queries Apr 1, 2021

revisions

e36fecd

AAfghahi force-pushed the saved_queries branch 2 times, most recently from 80a37cf to fd5f4b4 Compare April 6, 2021 15:36

started tests

cf68455

AAfghahi force-pushed the saved_queries branch from fd5f4b4 to cf68455 Compare April 6, 2021 15:37

added unit tests

a8416b0

AAfghahi marked this pull request as ready for review April 7, 2021 00:00

betodealmeida requested changes Apr 7, 2021

View reviewed changes

AAfghahi commented Apr 7, 2021

View reviewed changes

AAfghahi requested a review from betodealmeida April 7, 2021 17:50

AAfghahi commented Apr 7, 2021

View reviewed changes

revisions

b2ec820

AAfghahi force-pushed the saved_queries branch from 31aed8b to b2ec820 Compare April 7, 2021 19:24

betodealmeida approved these changes Apr 7, 2021

View reviewed changes

AAfghahi force-pushed the saved_queries branch from 72dccf0 to d453f12 Compare April 7, 2021 23:29

tests passing

710608d

AAfghahi force-pushed the saved_queries branch from d453f12 to 710608d Compare April 8, 2021 00:17

fixed api test

aec0aab

AAfghahi force-pushed the saved_queries branch from 46e01d3 to aec0aab Compare April 8, 2021 00:52

hughhhh reviewed Apr 8, 2021

View reviewed changes

superset/queries/saved_queries/commands/importers/v1/utils.py Show resolved Hide resolved

hughhhh approved these changes Apr 8, 2021

View reviewed changes

AAfghahi and others added 2 commits April 8, 2021 10:54

Update superset/queries/saved_queries/commands/importers/v1/utils.py

18580aa

Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>

Revert "Update superset/queries/saved_queries/commands/importers/v1/u…

a48bd8c

…tils.py" This reverts commit 18580aa.

AAfghahi force-pushed the saved_queries branch from 411171b to a48bd8c Compare April 8, 2021 16:37

hughhhh merged commit b5e5b3a into apache:master Apr 8, 2021

AAfghahi deleted the saved_queries branch April 8, 2021 18:23

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create backend routes and API for importing saved queries #13893

feat: create backend routes and API for importing saved queries #13893

AAfghahi commented Mar 31, 2021

betodealmeida Mar 31, 2021

AAfghahi Mar 31, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021 •

edited

Loading

AAfghahi Mar 31, 2021

betodealmeida Mar 31, 2021

AAfghahi Apr 1, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021 •

edited

Loading

AAfghahi Apr 1, 2021 •

edited

Loading

betodealmeida commented Mar 31, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021

betodealmeida Mar 31, 2021

codecov bot commented Apr 7, 2021 •

edited

Loading

betodealmeida Apr 7, 2021

betodealmeida Apr 7, 2021

betodealmeida Apr 7, 2021

betodealmeida Apr 7, 2021

AAfghahi Apr 7, 2021

betodealmeida Apr 7, 2021

AAfghahi Apr 7, 2021

betodealmeida Apr 7, 2021

betodealmeida Apr 7, 2021

AAfghahi Apr 7, 2021

betodealmeida Apr 7, 2021 •

edited

Loading

betodealmeida Apr 7, 2021

betodealmeida Apr 7, 2021

AAfghahi left a comment

AAfghahi left a comment

betodealmeida left a comment

AAfghahi commented Apr 7, 2021

		@@ -0,0 +1,56 @@
		# Licensed to the Apache Software Foundation (ASF) under one

	if file_name.startswith("queries/") and config["database_uuid"] in database:
	if file_name.startswith("queries/") and config["database_uuid"] in database_ids:

	"""Import Saved Queries with associated datasets and databases
	"""Import Saved Queries with associated databases

	description: overwrite existing databases?
	description: overwrite existing saved queries?

		assert saved_query.uuid == "05b679b5-8eaf-452c-b874-a7a774cfa4e9"
		assert saved_query.database_uuid == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89"

	"""Test different validations applied when importing a chart"""
	"""Test different validations applied when importing a saved query"""

feat: create backend routes and API for importing saved queries #13893

feat: create backend routes and API for importing saved queries #13893

Conversation

AAfghahi commented Mar 31, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida Mar 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida Mar 31, 2021 • edited Loading

Choose a reason for hiding this comment

AAfghahi Apr 1, 2021 • edited Loading

Choose a reason for hiding this comment

betodealmeida commented Mar 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 7, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida Apr 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AAfghahi left a comment

Choose a reason for hiding this comment

AAfghahi left a comment

Choose a reason for hiding this comment

betodealmeida left a comment

Choose a reason for hiding this comment

AAfghahi commented Apr 7, 2021

betodealmeida Mar 31, 2021 •

edited

Loading

betodealmeida Mar 31, 2021 •

edited

Loading

AAfghahi Apr 1, 2021 •

edited

Loading

codecov bot commented Apr 7, 2021 •

edited

Loading

betodealmeida Apr 7, 2021 •

edited

Loading