betodealmeida commented on a change in pull request #13893: URL: https://github.com/apache/superset/pull/13893#discussion_r608265831
########## File path: superset/queries/saved_queries/commands/importers/v1/__init__.py ########## @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any, Dict, Set + +from marshmallow import Schema +from sqlalchemy.orm import Session + +from superset.queries.saved_queries.commands.exceptions import SavedQueryImportError +from superset.commands.importers.v1 import ImportModelsCommand +from superset.connectors.sqla.models import SqlaTable +from superset.databases.commands.importers.v1.utils import import_database +from superset.datasets.commands.importers.v1.utils import import_dataset +from superset.datasets.schemas import ImportV1DatasetSchema +from superset.queries.saved_queries.dao import SavedQueryDAO +from superset.queries.saved_queries.commands.importers.v1.utils import import_saved_query +from superset.queries.saved_queries.schemas import ImportV1SavedQuerySchema + +class ImportSavedQueriesCommand(ImportModelsCommand): + """Import Saved Queries""" + + dao = SavedQueryDAO + model_name= "saved_queries" + prefix ="saved_queries/" + schemas: Dict[str, Schema] = { + "datasets/": ImportV1DatasetSchema(), + "queries/": ImportV1SavedQuerySchema(), + } + import_error = SavedQueryImportError + + @staticmethod + def _import( + session: Session, configs: Dict[str, Any], overwrite: bool = False + ) -> None: + # discover databases associated with saved queries + database_uuids: Set[str] = set() + for file_name, config in configs.items(): + if file_name.startswith("queries/"): + database_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.uid}) Review comment: A saved query has a many-to-one relationship to database; there are no datasources involved. What you need to do here is update the [`db_id` attribute of the saved query model](https://github.com/apache/superset/blob/master/superset/models/sql_lab.py#L179) and point it to the imported/existing database: ```suggestion config["db_id"] = database_ids[config["database_uuid"]] ``` ########## File path: superset/queries/saved_queries/commands/importers/v1/__init__.py ########## @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any, Dict, Set + +from marshmallow import Schema +from sqlalchemy.orm import Session + +from superset.queries.saved_queries.commands.exceptions import SavedQueryImportError +from superset.commands.importers.v1 import ImportModelsCommand +from superset.connectors.sqla.models import SqlaTable +from superset.databases.commands.importers.v1.utils import import_database +from superset.datasets.commands.importers.v1.utils import import_dataset +from superset.datasets.schemas import ImportV1DatasetSchema +from superset.queries.saved_queries.dao import SavedQueryDAO +from superset.queries.saved_queries.commands.importers.v1.utils import import_saved_query +from superset.queries.saved_queries.schemas import ImportV1SavedQuerySchema + +class ImportSavedQueriesCommand(ImportModelsCommand): + """Import Saved Queries""" + + dao = SavedQueryDAO + model_name= "saved_queries" + prefix ="saved_queries/" + schemas: Dict[str, Schema] = { + "datasets/": ImportV1DatasetSchema(), + "queries/": ImportV1SavedQuerySchema(), + } + import_error = SavedQueryImportError + + @staticmethod + def _import( + session: Session, configs: Dict[str, Any], overwrite: bool = False + ) -> None: + # discover databases associated with saved queries + database_uuids: Set[str] = set() + for file_name, config in configs.items(): + if file_name.startswith("queries/"): + database_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: Review comment: `database` here is a `Database` object, you want: ```suggestion if file_name.startswith("queries/") and config["database_uuid"] in database_ids: ``` ########## File path: tests/queries/saved_queries/commands_tests.py ########## @@ -108,3 +120,109 @@ def test_export_query_command_key_order(self, mock_g): "version", "database_uuid", ] +class TestImportSavedQueriesCommand(SupersetTestCase): + def test_import_v1_saved_queries(self): + """Test that we can import a saved query""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + + command = ImportSavedQueriesCommand(contents) + command.run() + + saved_query = db.session.query(SavedQuery).filter_by( + uuid=saved_queries_config["uuid"] + ).one() + assert saved_query.schema == "public" + 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 + """ + ) + assert saved_query.uuid == "05b679b5-8eaf-452c-b874-a7a774cfa4e9" + assert saved_query.database_uuid == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89" + + database = ( + db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() + ) + + db.session.delete(saved_query) + db.session.delete(database) + db.session.commit() + def test_import_v1_saved_queries_multiple(self): + """Test that a saved query can be imported multiple times""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + command = ImportSavedQueriesCommand(contents, overwrite=True) + command.run() + command.run() + database = ( + db.session.query(SavedQuery).filter_by(uuid=database_config["uuid"]).one() + ) + saved_query = db.session.query(SavedQuery).filter_by(datasource_id=database.id).all() + assert len(saved_query) == 1 + + db.session.delete(saved_query[0]) + db.session.delete(database) + db.session.commit() + + def test_import_v1_saved_queries_validation(self): + """Test different validations applied when importing a chart""" + # metadata.yaml must be present + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), Review comment: 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. ########## File path: tests/queries/saved_queries/commands_tests.py ########## @@ -108,3 +120,109 @@ def test_export_query_command_key_order(self, mock_g): "version", "database_uuid", ] +class TestImportSavedQueriesCommand(SupersetTestCase): + def test_import_v1_saved_queries(self): + """Test that we can import a saved query""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + + command = ImportSavedQueriesCommand(contents) + command.run() + + saved_query = db.session.query(SavedQuery).filter_by( + uuid=saved_queries_config["uuid"] + ).one() + assert saved_query.schema == "public" + 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 + """ + ) + assert saved_query.uuid == "05b679b5-8eaf-452c-b874-a7a774cfa4e9" + assert saved_query.database_uuid == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89" Review comment: I suspect these will change every time you run the tests, so you might need to remove these tests. ########## File path: superset/queries/saved_queries/commands/importers/v1/__init__.py ########## @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any, Dict, Set + +from marshmallow import Schema +from sqlalchemy.orm import Session + +from superset.queries.saved_queries.commands.exceptions import SavedQueryImportError +from superset.commands.importers.v1 import ImportModelsCommand +from superset.connectors.sqla.models import SqlaTable +from superset.databases.commands.importers.v1.utils import import_database +from superset.datasets.commands.importers.v1.utils import import_dataset +from superset.datasets.schemas import ImportV1DatasetSchema +from superset.queries.saved_queries.dao import SavedQueryDAO +from superset.queries.saved_queries.commands.importers.v1.utils import import_saved_query +from superset.queries.saved_queries.schemas import ImportV1SavedQuerySchema + +class ImportSavedQueriesCommand(ImportModelsCommand): + """Import Saved Queries""" + + dao = SavedQueryDAO + model_name= "saved_queries" + prefix ="saved_queries/" Review comment: This should be the subdirectory inside the ZIP where the relevant YAML files exist: ```suggestion prefix = "queries/" ``` ########## File path: tests/charts/commands_tests.py ########## @@ -197,7 +197,7 @@ def test_import_v1_chart(self): db.session.commit() def test_import_v1_chart_multiple(self): - """Test that a dataset can be imported multiple times""" + """Test that a chart can be imported multiple times""" Review comment: Thanks! :) ########## File path: superset/queries/saved_queries/api.py ########## @@ -252,3 +262,76 @@ def export(self, **kwargs: Any) -> Response: as_attachment=True, attachment_filename=filename, ) + @expose("/import/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", + log_to_statsd=False, + ) + def import_(self) -> Response: + """Import Saved Queries with associated datasets and databases + --- + post: + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + formData: + description: upload file (ZIP) + type: string + format: binary + passwords: + description: JSON map of passwords for each file + type: string + overwrite: + description: overwrite existing databases? Review comment: ```suggestion description: overwrite existing saved queries? ``` ########## File path: tests/queries/saved_queries/commands_tests.py ########## @@ -108,3 +120,109 @@ def test_export_query_command_key_order(self, mock_g): "version", "database_uuid", ] +class TestImportSavedQueriesCommand(SupersetTestCase): + def test_import_v1_saved_queries(self): + """Test that we can import a saved query""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + + command = ImportSavedQueriesCommand(contents) + command.run() + + saved_query = db.session.query(SavedQuery).filter_by( + uuid=saved_queries_config["uuid"] + ).one() + assert saved_query.schema == "public" + 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 + """ + ) + assert saved_query.uuid == "05b679b5-8eaf-452c-b874-a7a774cfa4e9" + assert saved_query.database_uuid == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89" + + database = ( + db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() + ) + + db.session.delete(saved_query) + db.session.delete(database) + db.session.commit() + def test_import_v1_saved_queries_multiple(self): + """Test that a saved query can be imported multiple times""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + command = ImportSavedQueriesCommand(contents, overwrite=True) + command.run() + command.run() + database = ( + db.session.query(SavedQuery).filter_by(uuid=database_config["uuid"]).one() + ) + saved_query = db.session.query(SavedQuery).filter_by(datasource_id=database.id).all() + assert len(saved_query) == 1 + + db.session.delete(saved_query[0]) + db.session.delete(database) + db.session.commit() + + def test_import_v1_saved_queries_validation(self): + """Test different validations applied when importing a chart""" Review comment: ```suggestion """Test different validations applied when importing a saved query""" ``` ########## File path: tests/queries/saved_queries/api_tests.py ########## @@ -745,3 +751,48 @@ def test_export_not_allowed(self): uri = f"api/v1/saved_query/export/?q={prison.dumps(argument)}" rv = self.client.get(uri) assert rv.status_code == 404 + + def create_saved_query_import(self): + buf = BytesIO() + with ZipFile(buf, "w") as bundle: + with bundle.open("saved_query_export/metadata.yaml", "w") as fp: + fp.write(yaml.safe_dump(saved_queries_metadata_config).encode()) + with bundle.open( + "saved_query_export/databases/imported_database.yaml", "w" + ) as fp: + fp.write(yaml.safe_dump(database_config).encode()) + with bundle.open("saved_query_export/queries/imported_database/public/imported_saved_query.yaml", "w") as fp: + fp.write(yaml.safe_dump(saved_queries_config).encode()) + buf.seek(0) + return buf + + @pytest.mark.usefixtures("create_saved_queries") Review comment: A [fixture](https://docs.pytest.org/en/stable/fixture.html) runs setup code before a test function, and cleanup code after. There's no fixture called `create_saved_queries`, so you should remove this. ########## File path: superset/queries/saved_queries/api.py ########## @@ -252,3 +262,76 @@ def export(self, **kwargs: Any) -> Response: as_attachment=True, attachment_filename=filename, ) + @expose("/import/", methods=["POST"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", + log_to_statsd=False, + ) + def import_(self) -> Response: + """Import Saved Queries with associated datasets and databases Review comment: ```suggestion """Import Saved Queries with associated databases ``` ########## File path: tests/queries/saved_queries/commands_tests.py ########## @@ -108,3 +120,109 @@ def test_export_query_command_key_order(self, mock_g): "version", "database_uuid", ] +class TestImportSavedQueriesCommand(SupersetTestCase): + def test_import_v1_saved_queries(self): + """Test that we can import a saved query""" + contents = { + "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(database_config), + "queries/imported_query.yaml": yaml.safe_dump(saved_queries_config) + } + + command = ImportSavedQueriesCommand(contents) + command.run() + + saved_query = db.session.query(SavedQuery).filter_by( + uuid=saved_queries_config["uuid"] + ).one() + assert saved_query.schema == "public" + 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 + """ + ) Review comment: This test will probably fail because the SQL is formatted differently (there's no line break after "your query,", eg). ########## File path: superset/queries/saved_queries/commands/importers/v1/__init__.py ########## @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any, Dict, Set + +from marshmallow import Schema +from sqlalchemy.orm import Session + +from superset.queries.saved_queries.commands.exceptions import SavedQueryImportError +from superset.commands.importers.v1 import ImportModelsCommand +from superset.connectors.sqla.models import SqlaTable +from superset.databases.commands.importers.v1.utils import import_database +from superset.datasets.commands.importers.v1.utils import import_dataset +from superset.datasets.schemas import ImportV1DatasetSchema +from superset.queries.saved_queries.dao import SavedQueryDAO +from superset.queries.saved_queries.commands.importers.v1.utils import import_saved_query +from superset.queries.saved_queries.schemas import ImportV1SavedQuerySchema + +class ImportSavedQueriesCommand(ImportModelsCommand): + """Import Saved Queries""" + + dao = SavedQueryDAO + model_name= "saved_queries" Review comment: 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`. ```suggestion model_name = "saved query" ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
