You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by dp...@apache.org on 2023/06/14 07:48:54 UTC

[superset] branch master updated: chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz (#24375)

This is an automated email from the ASF dual-hosted git repository.

dpgaspar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new c69634df27 chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz (#24375)
c69634df27 is described below

commit c69634df279054de0eb145bdcc796cb8fdd26dd8
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Wed Jun 14 08:48:29 2023 +0100

    chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz (#24375)
---
 RESOURCES/STANDARD_ROLES.md               |   1 -
 UPDATING.md                               |   1 +
 superset/security/manager.py              |   2 -
 superset/views/core.py                    | 129 +-----------------------------
 tests/integration_tests/security_tests.py |   2 -
 tests/integration_tests/sqllab_tests.py   |  97 ----------------------
 6 files changed, 2 insertions(+), 230 deletions(-)

diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md
index 27af7bba2f..cbef53abbe 100644
--- a/RESOURCES/STANDARD_ROLES.md
+++ b/RESOURCES/STANDARD_ROLES.md
@@ -64,7 +64,6 @@
 |can user slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
 |can favstar on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
 |can import dashboards on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
-|can sqllab viz on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|
 |can schemas on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
 |can sqllab history on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|
 |can publish on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
diff --git a/UPDATING.md b/UPDATING.md
index 72c9f7d6bf..3db0ab6a4b 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -34,6 +34,7 @@ assists people when migrating to a new version.
 
 ### Breaking Changes
 
+- [24375](https://github.com/apache/superset/pull/24375): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz`
 - [24360](https://github.com/apache/superset/pull/24360): Removed deprecated APIs `/superset/stop_query/...`, `/superset/queries/...`, `/superset/search_queries`
 - [24353](https://github.com/apache/superset/pull/24353): Removed deprecated APIs `/copy_dash/int:dashboard_id/`, `/save_dash/int:dashboard_id/`, `/add_slices/int:dashboard_id/`.
 - [24198](https://github.com/apache/superset/pull/24198) The FAB views `User Registrations` and `User's Statistics` have been changed to Admin only. To re-enable them for non-admin users, please add the following perms to your custom role: `menu access on User's Statistics` and `menu access on User Registrations`.
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 41e522f3ea..1e01bc7417 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -238,8 +238,6 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
         ("can_estimate_query_cost", "SQL Lab"),
         ("can_export_csv", "SQLLab"),
         ("can_sqllab_history", "Superset"),
-        ("can_sqllab_viz", "Superset"),
-        ("can_sqllab_table_viz", "Superset"),  # Deprecated permission remove on 3.0.0
         ("can_sqllab", "Superset"),
         ("can_test_conn", "Superset"),  # Deprecated permission remove on 3.0.0
         ("can_activate", "TabStateView"),
diff --git a/superset/views/core.py b/superset/views/core.py
index 8483733730..782fad4978 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -49,12 +49,7 @@ from superset.charts.commands.exceptions import ChartNotFoundError
 from superset.charts.dao import ChartDAO
 from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType
 from superset.connectors.base.models import BaseDatasource
-from superset.connectors.sqla.models import (
-    AnnotationDatasource,
-    SqlaTable,
-    SqlMetric,
-    TableColumn,
-)
+from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable
 from superset.dashboards.commands.exceptions import DashboardAccessDeniedError
 from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand
 from superset.dashboards.permalink.commands.get import GetDashboardPermalinkCommand
@@ -62,13 +57,10 @@ from superset.dashboards.permalink.exceptions import DashboardPermalinkGetFailed
 from superset.databases.dao import DatabaseDAO
 from superset.datasets.commands.exceptions import DatasetNotFoundError
 from superset.datasource.dao import DatasourceDAO
-from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
     CacheLoadError,
     DatabaseNotFound,
-    SupersetErrorException,
     SupersetException,
-    SupersetGenericErrorException,
     SupersetSecurityException,
 )
 from superset.explore.form_data.commands.create import CreateFormDataCommand
@@ -82,7 +74,6 @@ from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
 from superset.models.sql_lab import Query, TabState
 from superset.models.user_attributes import UserAttribute
-from superset.sql_parse import ParsedQuery
 from superset.superset_typing import FlaskResponse
 from superset.tasks.async_queries import load_explore_json_into_cache
 from superset.utils import core as utils
@@ -100,9 +91,7 @@ from superset.views.base import (
     get_error_msg,
     handle_api_exception,
     json_error_response,
-    json_errors_response,
     json_success,
-    validate_sqlatable,
 )
 from superset.views.log.dao import LogDAO
 from superset.views.utils import (
@@ -1290,122 +1279,6 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
     def log(self) -> FlaskResponse:  # pylint: disable=no-self-use
         return Response(status=200)
 
-    @has_access
-    @expose("/get_or_create_table/", methods=("POST",))
-    @event_logger.log_this
-    @deprecated(new_target="api/v1/dataset/get_or_create/")
-    def sqllab_table_viz(self) -> FlaskResponse:  # pylint: disable=no-self-use
-        """Gets or creates a table object with attributes passed to the API.
-
-        It expects the json with params:
-        * datasourceName - e.g. table name, required
-        * dbId - database id, required
-        * schema - table schema, optional
-        * templateParams - params for the Jinja templating syntax, optional
-        :return: Response
-        """
-        data = json.loads(request.form["data"])
-        table_name = data["datasourceName"]
-        database_id = data["dbId"]
-        table = (
-            db.session.query(SqlaTable)
-            .filter_by(database_id=database_id, table_name=table_name)
-            .one_or_none()
-        )
-        if not table:
-            # Create table if doesn't exist.
-            with db.session.no_autoflush:
-                table = SqlaTable(table_name=table_name, owners=[g.user])
-                table.database_id = database_id
-                table.database = (
-                    db.session.query(Database).filter_by(id=database_id).one()
-                )
-                table.schema = data.get("schema")
-                table.template_params = data.get("templateParams")
-                # needed for the table validation.
-                # fn can be deleted when this endpoint is removed
-                validate_sqlatable(table)
-
-            db.session.add(table)
-            table.fetch_metadata()
-            db.session.commit()
-
-        return json_success(json.dumps({"table_id": table.id}))
-
-    @has_access
-    @expose("/sqllab_viz/", methods=("POST",))
-    @event_logger.log_this
-    @deprecated(new_target="api/v1/dataset/")
-    def sqllab_viz(self) -> FlaskResponse:  # pylint: disable=no-self-use
-        data = json.loads(request.form["data"])
-        try:
-            table_name = data["datasourceName"]
-            database_id = data["dbId"]
-        except KeyError as ex:
-            raise SupersetGenericErrorException(
-                __(
-                    "One or more required fields are missing in the request. Please try "
-                    "again, and if the problem persists contact your administrator."
-                ),
-                status=400,
-            ) from ex
-        database = db.session.query(Database).get(database_id)
-        if not database:
-            raise SupersetErrorException(
-                SupersetError(
-                    message=__("The database was not found."),
-                    error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR,
-                    level=ErrorLevel.ERROR,
-                ),
-                status=404,
-            )
-        table = (
-            db.session.query(SqlaTable)
-            .filter_by(database_id=database_id, table_name=table_name)
-            .one_or_none()
-        )
-
-        if table:
-            return json_errors_response(
-                [
-                    SupersetError(
-                        message=f"Dataset [{table_name}] already exists",
-                        error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
-                        level=ErrorLevel.WARNING,
-                    )
-                ],
-                status=422,
-            )
-
-        table = SqlaTable(table_name=table_name, owners=[g.user])
-        table.database = database
-        table.schema = data.get("schema")
-        table.template_params = data.get("templateParams")
-        table.is_sqllab_view = True
-        table.sql = ParsedQuery(data.get("sql")).stripped()
-        db.session.add(table)
-        cols = []
-        for config_ in data.get("columns"):
-            column_name = config_.get("column_name") or config_.get("name")
-            col = TableColumn(
-                column_name=column_name,
-                filterable=True,
-                groupby=True,
-                is_dttm=config_.get("is_dttm", False),
-                type=config_.get("type", False),
-            )
-            cols.append(col)
-
-        table.columns = cols
-        table.metrics = [SqlMetric(metric_name="count", expression="count(*)")]
-        db.session.commit()
-
-        return json_success(
-            json.dumps(
-                {"table_id": table.id, "data": sanitize_datasource_data(table.data)}
-            )
-        )
-
     @has_access
     @expose("/extra_table_metadata/<int:database_id>/<table_name>/<schema>/")
     @event_logger.log_this
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index e5b376dc24..e7b85498a5 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -1497,8 +1497,6 @@ class TestRolePermission(SupersetTestCase):
         self.assertIn(("can_csv", "Superset"), sql_lab_set)
         self.assertIn(("can_read", "Database"), sql_lab_set)
         self.assertIn(("can_read", "SavedQuery"), sql_lab_set)
-        self.assertIn(("can_sqllab_viz", "Superset"), sql_lab_set)
-        self.assertIn(("can_sqllab_table_viz", "Superset"), sql_lab_set)
         self.assertIn(("can_sqllab", "Superset"), sql_lab_set)
 
         self.assertIn(("menu_access", "SQL Lab"), sql_lab_set)
diff --git a/tests/integration_tests/sqllab_tests.py b/tests/integration_tests/sqllab_tests.py
index 6880fb2325..fbab4d98d2 100644
--- a/tests/integration_tests/sqllab_tests.py
+++ b/tests/integration_tests/sqllab_tests.py
@@ -22,7 +22,6 @@ from datetime import datetime
 import pytest
 from celery.exceptions import SoftTimeLimitExceeded
 from parameterized import parameterized
-from random import random
 from unittest import mock
 import prison
 
@@ -358,102 +357,6 @@ class TestSqlLab(SupersetTestCase):
         self.assertEqual(len(data), results.size)
         self.assertEqual(len(cols), len(results.columns))
 
-    def test_sqllab_viz(self):
-        self.login("admin")
-        examples_dbid = get_example_database().id
-        payload = {
-            "chartType": "dist_bar",
-            "datasourceName": f"test_viz_flow_table_{random()}",
-            "schema": "superset",
-            "columns": [
-                {
-                    "is_dttm": False,
-                    "type": "STRING",
-                    "column_name": f"viz_type_{random()}",
-                },
-                {
-                    "is_dttm": False,
-                    "type": "OBJECT",
-                    "column_name": f"ccount_{random()}",
-                },
-            ],
-            "sql": """\
-                SELECT *
-                FROM birth_names
-                LIMIT 10""",
-            "dbId": examples_dbid,
-        }
-        data = {"data": json.dumps(payload)}
-        resp = self.get_json_resp("/superset/sqllab_viz/", data=data)
-        self.assertIn("table_id", resp)
-
-        # ensure owner is set correctly
-        table_id = resp["table_id"]
-        table = db.session.query(SqlaTable).filter_by(id=table_id).one()
-        self.assertEqual([owner.username for owner in table.owners], ["admin"])
-        view_menu = security_manager.find_view_menu(table.get_perm())
-        assert view_menu is not None
-
-        # Cleanup
-        db.session.delete(table)
-        db.session.commit()
-
-    def test_sqllab_viz_bad_payload(self):
-        self.login("admin")
-        payload = {
-            "chartType": "dist_bar",
-            "schema": "superset",
-            "columns": [
-                {
-                    "is_dttm": False,
-                    "type": "STRING",
-                    "column_name": f"viz_type_{random()}",
-                },
-                {
-                    "is_dttm": False,
-                    "type": "OBJECT",
-                    "column_name": f"ccount_{random()}",
-                },
-            ],
-            "sql": """\
-                SELECT *
-                FROM birth_names
-                LIMIT 10""",
-        }
-        data = {"data": json.dumps(payload)}
-        url = "/superset/sqllab_viz/"
-        response = self.client.post(url, data=data, follow_redirects=True)
-        assert response.status_code == 400
-
-    def test_sqllab_table_viz(self):
-        self.login("admin")
-        examples_db = get_example_database()
-        with examples_db.get_sqla_engine_with_context() as engine:
-            engine.execute("DROP TABLE IF EXISTS test_sqllab_table_viz")
-            engine.execute("CREATE TABLE test_sqllab_table_viz AS SELECT 2 as col")
-
-        examples_dbid = examples_db.id
-
-        payload = {
-            "datasourceName": "test_sqllab_table_viz",
-            "columns": [],
-            "dbId": examples_dbid,
-        }
-
-        data = {"data": json.dumps(payload)}
-        resp = self.get_json_resp("/superset/get_or_create_table/", data=data)
-        self.assertIn("table_id", resp)
-
-        # ensure owner is set correctly
-        table_id = resp["table_id"]
-        table = db.session.query(SqlaTable).filter_by(id=table_id).one()
-        self.assertEqual([owner.username for owner in table.owners], ["admin"])
-        db.session.delete(table)
-
-        with get_example_database().get_sqla_engine_with_context() as engine:
-            engine.execute("DROP TABLE test_sqllab_table_viz")
-        db.session.commit()
-
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_sql_limit(self):
         self.login("admin")