From 0968f86584ba751bd665bb3e684b7aae0a28fa60 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 29 Mar 2022 13:23:05 -0700 Subject: [PATCH] chore: remove deprecated config keys and endpoints code 2.0 (#19361) * remove depracted keys and associated methods * remove api methods and tablemodel views * remove deprecated api and unsued vars * reremove schedules * readd code * fix pylint * run black * remove test * core select start test * add suggested changes --- docker/pythonpath_dev/superset_config.py | 2 +- superset/config.py | 50 +------- superset/connectors/sqla/views.py | 146 +---------------------- superset/views/core.py | 137 +-------------------- tests/integration_tests/core_tests.py | 16 --- 5 files changed, 10 insertions(+), 341 deletions(-) diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py index 6c58bec79c..794239d23f 100644 --- a/docker/pythonpath_dev/superset_config.py +++ b/docker/pythonpath_dev/superset_config.py @@ -72,7 +72,7 @@ RESULTS_BACKEND = FileSystemCache("/app/superset_home/sqllab") class CeleryConfig(object): BROKER_URL = f"redis://{REDIS_HOST}:{REDIS_PORT}/{REDIS_CELERY_DB}" - CELERY_IMPORTS = ("superset.sql_lab", "superset.tasks") + CELERY_IMPORTS = ("superset.sql_lab",) CELERY_RESULT_BACKEND = f"redis://{REDIS_HOST}:{REDIS_PORT}/{REDIS_RESULTS_DB}" CELERYD_LOG_LEVEL = "DEBUG" CELERYD_PREFETCH_MULTIPLIER = 1 diff --git a/superset/config.py b/superset/config.py index 80224a68c3..4c68aa2551 100644 --- a/superset/config.py +++ b/superset/config.py @@ -743,7 +743,7 @@ DASHBOARD_AUTO_REFRESH_MODE: Literal["fetch", "force"] = "force" class CeleryConfig: # pylint: disable=too-few-public-methods broker_url = "sqla+sqlite:///celerydb.sqlite" - imports = ("superset.sql_lab", "superset.tasks") + imports = ("superset.sql_lab",) result_backend = "db+sqlite:///celery_results.sqlite" worker_log_level = "DEBUG" worker_prefetch_multiplier = 1 @@ -1051,6 +1051,11 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument return sql +# This auth provider is used by background (offline) tasks that need to access +# protected resources. Can be overridden by end users in order to support +# custom auth mechanisms +MACHINE_AUTH_PROVIDER_CLASS = "superset.utils.machine_auth.MachineAuthProvider" + # --------------------------------------------------- # Alerts & Reports # --------------------------------------------------- @@ -1075,43 +1080,6 @@ EMAIL_REPORTS_SUBJECT_PREFIX = "[Report] " SLACK_API_TOKEN: Optional[Union[Callable[[], str], str]] = None SLACK_PROXY = None -# If enabled, certain features are run in debug mode -# Current list: -# * Emails are sent using dry-run mode (logging only) -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -SCHEDULED_EMAIL_DEBUG_MODE = False - -# This auth provider is used by background (offline) tasks that need to access -# protected resources. Can be overridden by end users in order to support -# custom auth mechanisms -MACHINE_AUTH_PROVIDER_CLASS = "superset.utils.machine_auth.MachineAuthProvider" - -# Email reports - minimum time resolution (in minutes) for the crontab -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -EMAIL_REPORTS_CRON_RESOLUTION = 15 - -# The MAX duration (in seconds) a email schedule can run for before being killed -# by celery. -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -EMAIL_ASYNC_TIME_LIMIT_SEC = int(timedelta(minutes=5).total_seconds()) - -# Send bcc of all reports to this address. Set to None to disable. -# This is useful for maintaining an audit trail of all email deliveries. -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -EMAIL_REPORT_BCC_ADDRESS = None - -# User credentials to use for generating reports -# This user should have permissions to browse all the dashboards and -# slices. -# TODO: In the future, login as the owner of the item to generate reports -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -EMAIL_REPORTS_USER = "admin" - # The webdriver to use for generating reports. Use one of the following # firefox # Requires: geckodriver and firefox installations @@ -1238,12 +1206,6 @@ PREVENT_UNSAFE_DB_CONNECTIONS = True # Example: SSL_CERT_PATH = "/certs" SSL_CERT_PATH: Optional[str] = None -# Turn this key to False to disable ownership check on the old dataset MVC and -# datasource API /datasource/save. -# -# Warning: This config key is deprecated and will be removed in version 2.0.0" -OLD_API_CHECK_DATASET_OWNERSHIP = True - # SQLA table mutator, every time we fetch the metadata for a certain table # (superset.connectors.sqla.models.SqlaTable), we call this hook # to allow mutating the object with this callback. diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 4b43d2f046..116b18869d 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -17,17 +17,15 @@ """Views used by the SqlAlchemy connector""" import logging import re -from dataclasses import dataclass, field -from typing import Any, cast, Dict, List, Union +from typing import Any, cast from flask import current_app, flash, Markup, redirect from flask_appbuilder import CompactCRUDMixin, expose -from flask_appbuilder.actions import action from flask_appbuilder.fieldwidgets import Select2Widget from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.decorators import has_access -from flask_babel import gettext as __, lazy_gettext as _ +from flask_babel import lazy_gettext as _ from werkzeug.exceptions import NotFound from wtforms.ext.sqlalchemy.fields import QuerySelectField from wtforms.validators import Regexp @@ -39,14 +37,12 @@ from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.superset_typing import FlaskResponse from superset.utils import core as utils from superset.views.base import ( - check_ownership, create_table_permissions, DatasourceFilter, DeleteMixin, ListWidgetWithCheckboxes, SupersetListWidget, SupersetModelView, - validate_sqlatable, YamlExportMixin, ) @@ -181,27 +177,6 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView): edit_form_extra_fields = add_form_extra_fields - def pre_add(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - - def pre_update(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - - def pre_delete(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): datamodel = SQLAInterface(models.SqlMetric) @@ -274,27 +249,6 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): edit_form_extra_fields = add_form_extra_fields - def pre_add(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - - def pre_update(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - - def pre_delete(self, item: "models.SqlMetric") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item.table) - class RowLevelSecurityListWidget( SupersetListWidget @@ -513,19 +467,6 @@ class TableModelView( # pylint: disable=too-many-ancestors ) } - def pre_add(self, item: "TableModelView") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - validate_sqlatable(item) - - def pre_update(self, item: "TableModelView") -> None: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: - check_ownership(item) - def post_add( # pylint: disable=arguments-differ self, item: "TableModelView", @@ -561,89 +502,6 @@ class TableModelView( # pylint: disable=too-many-ancestors return resp return redirect("/superset/explore/table/{}/".format(pk)) - @action( - "refresh", __("Refresh Metadata"), __("Refresh column metadata"), "fa-refresh" - ) - def refresh( # pylint: disable=no-self-use, - self, tables: Union["TableModelView", List["TableModelView"]] - ) -> FlaskResponse: - logger.warning( - "This endpoint is deprecated and will be removed in version 2.0.0" - ) - if not isinstance(tables, list): - tables = [tables] - - @dataclass - class RefreshResults: - successes: List[TableModelView] = field(default_factory=list) - failures: List[TableModelView] = field(default_factory=list) - added: Dict[str, List[str]] = field(default_factory=dict) - removed: Dict[str, List[str]] = field(default_factory=dict) - modified: Dict[str, List[str]] = field(default_factory=dict) - - results = RefreshResults() - - for table_ in tables: - try: - metadata_results = table_.fetch_metadata() - if metadata_results.added: - results.added[table_.table_name] = metadata_results.added - if metadata_results.removed: - results.removed[table_.table_name] = metadata_results.removed - if metadata_results.modified: - results.modified[table_.table_name] = metadata_results.modified - results.successes.append(table_) - except Exception: # pylint: disable=broad-except - results.failures.append(table_) - - if len(results.successes) > 0: - success_msg = _( - "Metadata refreshed for the following table(s): %(tables)s", - tables=", ".join([t.table_name for t in results.successes]), - ) - flash(success_msg, "info") - if results.added: - added_tables = [] - for table, cols in results.added.items(): - added_tables.append(f"{table} ({', '.join(cols)})") - flash( - _( - "The following tables added new columns: %(tables)s", - tables=", ".join(added_tables), - ), - "info", - ) - if results.removed: - removed_tables = [] - for table, cols in results.removed.items(): - removed_tables.append(f"{table} ({', '.join(cols)})") - flash( - _( - "The following tables removed columns: %(tables)s", - tables=", ".join(removed_tables), - ), - "info", - ) - if results.modified: - modified_tables = [] - for table, cols in results.modified.items(): - modified_tables.append(f"{table} ({', '.join(cols)})") - flash( - _( - "The following tables update column metadata: %(tables)s", - tables=", ".join(modified_tables), - ), - "info", - ) - if len(results.failures) > 0: - failure_msg = _( - "Unable to refresh metadata for the following table(s): %(tables)s", - tables=", ".join([t.table_name for t in results.failures]), - ) - flash(failure_msg, "danger") - - return redirect("/tablemodelview/list/") - @expose("/list/") @has_access def list(self) -> FlaskResponse: diff --git a/superset/views/core.py b/superset/views/core.py index 5f6160642d..c806470e8f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -104,7 +104,7 @@ from superset.models.user_attributes import UserAttribute from superset.queries.dao import QueryDAO from superset.security.analytics_db_safety import check_sqlalchemy_uri from superset.sql_lab import get_sql_results -from superset.sql_parse import ParsedQuery, Table +from superset.sql_parse import ParsedQuery from superset.sql_validators import get_validator_by_name from superset.sqllab.command import CommandResult, ExecuteSqlCommand from superset.sqllab.command_status import SqlJsonExecutionStatus @@ -1084,31 +1084,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods return json_success(json.dumps(response)) - @api - @has_access_api - @event_logger.log_this - @expose("/schemas//") - @expose("/schemas///") - def schemas( # pylint: disable=no-self-use - self, db_id: int, force_refresh: str = "false" - ) -> FlaskResponse: - logger.warning( - "This API endpoint is deprecated and will be removed in version 2.0.0" - ) - db_id = int(db_id) - database = db.session.query(Database).get(db_id) - if database: - schemas = database.get_all_schema_names( - cache=database.schema_cache_enabled, - cache_timeout=database.schema_cache_timeout, - force=force_refresh.lower() == "true", - ) - schemas = security_manager.get_schemas_accessible_by_user(database, schemas) - else: - schemas = [] - - return Response(json.dumps({"schemas": schemas}), mimetype="application/json") - @api @has_access_api @event_logger.log_this @@ -1547,18 +1522,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods ) return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - @api - @has_access_api - @event_logger.log_this - @expose("/csrf_token/", methods=["GET"]) - def csrf_token(self) -> FlaskResponse: - logger.warning( - "This API endpoint is deprecated and will be removed in version 2.0.0" - ) - return Response( - self.render_template("superset/csrf_token.json"), mimetype="text/json" - ) - @api @has_access_api @event_logger.log_this @@ -1902,47 +1865,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods session.commit() return json_success(json.dumps({"count": count})) - @api - @has_access_api - @event_logger.log_this - @expose("/dashboard//published/", methods=("GET", "POST")) - def publish( # pylint: disable=no-self-use - self, dashboard_id: int - ) -> FlaskResponse: - """Gets and toggles published status on dashboards""" - logger.warning( - "This API endpoint is deprecated and will be removed in version 2.0.0" - ) - session = db.session() - Role = ab_models.Role - dash = ( - session.query(Dashboard).filter(Dashboard.id == dashboard_id).one_or_none() - ) - admin_role = session.query(Role).filter(Role.name == "Admin").one_or_none() - - if request.method == "GET": - if dash: - return json_success(json.dumps({"published": dash.published})) - - return json_error_response( - f"ERROR: cannot find dashboard {dashboard_id}", status=404 - ) - - edit_perm = ( - is_owner(dash, g.user) or admin_role in security_manager.get_user_roles() - ) - if not edit_perm: - username = g.user.username if hasattr(g.user, "username") else "user" - return json_error_response( - f'ERROR: "{username}" cannot alter ' - f'dashboard "{dash.dashboard_title}"', - status=403, - ) - - dash.published = str(request.form["published"]).lower() == "true" - session.commit() - return json_success(json.dumps({"published": dash.published})) - @has_access @expose("/dashboard//") @event_logger.log_this_with_extra_payload @@ -2211,63 +2133,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods ) return json_success(json.dumps(payload)) - @has_access - @expose("/select_star//") - @expose("/select_star///") - @event_logger.log_this - def select_star( - self, database_id: int, table_name: str, schema: Optional[str] = None - ) -> FlaskResponse: - logging.warning( - "%s.select_star " - "This API endpoint is deprecated and will be removed in version 2.0.0", - self.__class__.__name__, - ) - stats_logger.incr(f"{self.__class__.__name__}.select_star.init") - database = db.session.query(Database).get(database_id) - if not database: - stats_logger.incr( - f"deprecated.{self.__class__.__name__}.select_star.database_not_found" - ) - raise SupersetErrorException( - SupersetError( - message=__("The database was not found."), - error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR, - level=ErrorLevel.ERROR, - ), - status=404, - ) - schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) - table_name = utils.parse_js_uri_path_item(table_name) # type: ignore - if not self.appbuilder.sm.can_access_table(database, Table(table_name, schema)): - stats_logger.incr( - f"deprecated.{self.__class__.__name__}.select_star.permission_denied" - ) - logging.warning( - "Permission denied for user %s on table: %s schema: %s", - str(g.user), - table_name, - schema, - ) - raise SupersetErrorException( - SupersetError( - message=__( - "You are not authorized to fetch samples from this table. If " - "you think this is an error, please reach out to your " - "administrator." - ), - error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR, - level=ErrorLevel.ERROR, - ), - status=403, - ) - stats_logger.incr(f"deprecated.{self.__class__.__name__}.select_star.success") - return json_success( - database.select_star( - table_name, schema, latest_partition=True, show_cols=True - ) - ) - @has_access_api @expose("/estimate_query_cost//", methods=["POST"]) @expose("/estimate_query_cost///", methods=["POST"]) diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index ca0b374e97..166abf96a7 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -1240,22 +1240,6 @@ class TestCore(SupersetTestCase): assert data == ["this_schema_is_allowed_too"] self.delete_fake_db() - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_select_star(self): - self.login(username="admin") - examples_db = superset.utils.database.get_example_database() - resp = self.get_resp(f"/superset/select_star/{examples_db.id}/birth_names") - self.assertIn("gender", resp) - - def test_get_select_star_not_allowed(self): - """ - Database API: Test get select star not allowed - """ - self.login(username="gamma") - example_db = superset.utils.database.get_example_database() - resp = self.client.get(f"/superset/select_star/{example_db.id}/birth_names") - self.assertEqual(resp.status_code, 403) - @mock.patch("superset.views.core.results_backend_use_msgpack", False) def test_display_limit(self): from superset.views import core