From 56e6c1a2e1a288edf5618474e8a185698998ecbe Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 15 Oct 2019 08:21:37 -0700 Subject: [PATCH 01/42] First cut at app factory --- superset/__init__.py | 236 +++----------------- superset/app.py | 251 ++++++++++++++++++++++ superset/connectors/sqla/models.py | 3 +- superset/connectors/sqla/views.py | 2 +- superset/db_engine_specs/base.py | 7 +- superset/db_engine_specs/hive.py | 9 +- superset/db_engine_specs/presto.py | 13 +- superset/extensions.py | 43 ++++ superset/jinja_context.py | 6 +- superset/models/core.py | 9 +- superset/models/schedules.py | 2 +- superset/models/sql_lab.py | 2 +- superset/models/user_attributes.py | 2 +- superset/utils/cache.py | 12 +- superset/utils/cache_manager.py | 40 ++++ superset/utils/core.py | 14 -- superset/utils/feature_flag_manager.py | 40 ++++ superset/utils/manifest_processor.py | 69 ++++++ superset/utils/results_backend_manager.py | 35 +++ superset/viz.py | 21 +- 20 files changed, 549 insertions(+), 267 deletions(-) create mode 100644 superset/app.py create mode 100644 superset/extensions.py create mode 100644 superset/utils/cache_manager.py create mode 100644 superset/utils/feature_flag_manager.py create mode 100644 superset/utils/manifest_processor.py create mode 100644 superset/utils/results_backend_manager.py diff --git a/superset/__init__.py b/superset/__init__.py index 2d89234d93102..9c1447fe89ba4 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -14,228 +14,42 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# pylint: disable=C,R,W """Package's main module!""" -from copy import deepcopy -import json -import logging -import os +from flask import current_app +from werkzeug.local import LocalProxy -from flask import Flask, redirect -from flask_appbuilder import AppBuilder, IndexView, SQLA -from flask_appbuilder.baseviews import expose -from flask_compress import Compress -from flask_migrate import Migrate -from flask_talisman import Talisman -from flask_wtf.csrf import CSRFProtect -import wtforms_json - -from superset import config +from superset.app import create_app from superset.connectors.connector_registry import ConnectorRegistry +from superset.extensions import appbuilder from superset.security import SupersetSecurityManager -from superset.utils.core import pessimistic_connection_handling, setup_cache from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value -wtforms_json.init() - -APP_DIR = os.path.dirname(__file__) -CONFIG_MODULE = os.environ.get("SUPERSET_CONFIG", "superset.config") - -if not os.path.exists(config.DATA_DIR): - os.makedirs(config.DATA_DIR) - -app = Flask(__name__) -app.config.from_object(CONFIG_MODULE) -conf = app.config - -################################################################# -# Handling manifest file logic at app start -################################################################# -MANIFEST_FILE = APP_DIR + "/static/assets/dist/manifest.json" -manifest = {} - - -def parse_manifest_json(): - global manifest - try: - with open(MANIFEST_FILE, "r") as f: - # the manifest inclues non-entry files - # we only need entries in templates - full_manifest = json.load(f) - manifest = full_manifest.get("entrypoints", {}) - except Exception: - pass - - -def get_js_manifest_files(filename): - if app.debug: - parse_manifest_json() - entry_files = manifest.get(filename, {}) - return entry_files.get("js", []) - - -def get_css_manifest_files(filename): - if app.debug: - parse_manifest_json() - entry_files = manifest.get(filename, {}) - return entry_files.get("css", []) - - -def get_unloaded_chunks(files, loaded_chunks): - filtered_files = [f for f in files if f not in loaded_chunks] - for f in filtered_files: - loaded_chunks.add(f) - return filtered_files - - -parse_manifest_json() - - -@app.context_processor -def get_manifest(): - return dict( - loaded_chunks=set(), - get_unloaded_chunks=get_unloaded_chunks, - js_manifest=get_js_manifest_files, - css_manifest=get_css_manifest_files, - ) - - -################################################################# - -for bp in conf.get("BLUEPRINTS"): - try: - print("Registering blueprint: '{}'".format(bp.name)) - app.register_blueprint(bp) - except Exception as e: - print("blueprint registration failed") - logging.exception(e) - -if conf.get("SILENCE_FAB"): - logging.getLogger("flask_appbuilder").setLevel(logging.ERROR) - -db = SQLA(app) - -if conf.get("WTF_CSRF_ENABLED"): - csrf = CSRFProtect(app) - csrf_exempt_list = conf.get("WTF_CSRF_EXEMPT_LIST", []) - for ex in csrf_exempt_list: - csrf.exempt(ex) - -pessimistic_connection_handling(db.engine) - -cache = setup_cache(app, conf.get("CACHE_CONFIG")) -tables_cache = setup_cache(app, conf.get("TABLE_NAMES_CACHE_CONFIG")) - -migrate = Migrate(app, db, directory=APP_DIR + "/migrations") - -app.config.get("LOGGING_CONFIGURATOR").configure_logging(app.config, app.debug) - -if app.config.get("ENABLE_CORS"): - from flask_cors import CORS - - CORS(app, **app.config.get("CORS_OPTIONS")) - -if app.config.get("ENABLE_PROXY_FIX"): - from werkzeug.middleware.proxy_fix import ProxyFix - - app.wsgi_app = ProxyFix(app.wsgi_app, **app.config.get("PROXY_FIX_CONFIG")) - -if app.config.get("ENABLE_CHUNK_ENCODING"): - - class ChunkedEncodingFix(object): - def __init__(self, app): - self.app = app - - def __call__(self, environ, start_response): - # Setting wsgi.input_terminated tells werkzeug.wsgi to ignore - # content-length and read the stream till the end. - if environ.get("HTTP_TRANSFER_ENCODING", "").lower() == u"chunked": - environ["wsgi.input_terminated"] = True - return self.app(environ, start_response) - - app.wsgi_app = ChunkedEncodingFix(app.wsgi_app) - -if app.config.get("UPLOAD_FOLDER"): - try: - os.makedirs(app.config.get("UPLOAD_FOLDER")) - except OSError: - pass - -for middleware in app.config.get("ADDITIONAL_MIDDLEWARE"): - app.wsgi_app = middleware(app.wsgi_app) - - -class MyIndexView(IndexView): - @expose("/") - def index(self): - return redirect("/superset/welcome") - - -custom_sm = app.config.get("CUSTOM_SECURITY_MANAGER") or SupersetSecurityManager -if not issubclass(custom_sm, SupersetSecurityManager): - raise Exception( - """Your CUSTOM_SECURITY_MANAGER must now extend SupersetSecurityManager, - not FAB's security manager. - See [4565] in UPDATING.md""" - ) - -with app.app_context(): - appbuilder = AppBuilder( - app, - db.session, - base_template="superset/base.html", - indexview=MyIndexView, - security_manager_class=custom_sm, - update_perms=False, # Run `superset init` to update FAB's perms - ) - -security_manager = appbuilder.sm - -results_backend = app.config.get("RESULTS_BACKEND") -results_backend_use_msgpack = app.config.get("RESULTS_BACKEND_USE_MSGPACK") - -# Merge user defined feature flags with default feature flags -_feature_flags = app.config.get("DEFAULT_FEATURE_FLAGS") or {} -_feature_flags.update(app.config.get("FEATURE_FLAGS") or {}) - -# Event Logger -event_logger = get_event_logger_from_cfg_value( - app.config.get("EVENT_LOGGER", DBEventLogger()) -) - - -def get_feature_flags(): - GET_FEATURE_FLAGS_FUNC = app.config.get("GET_FEATURE_FLAGS_FUNC") - if GET_FEATURE_FLAGS_FUNC: - return GET_FEATURE_FLAGS_FUNC(deepcopy(_feature_flags)) - return _feature_flags - - -def is_feature_enabled(feature): - """Utility function for checking whether a feature is turned on""" - return get_feature_flags().get(feature) +appbuilder = extensions.appbuilder +cache = LocalProxy(lambda: extensions.cache_manager.cache) +conf = LocalProxy(lambda: current_app.config) +db = extensions.db +event_logger = extensions.event_logger +get_feature_flags = extensions.feature_flag_manager.get_feature_flags +is_feature_enabled = extensions.feature_flag_manager.is_feature_enabled +results_backend = LocalProxy(lambda: extensions.results_backend_manager.results_backend) +results_backend_use_msgpack = LocalProxy(lambda: extensions.results_backend_manager.should_use_msgpack) +security_manager = extensions.security_manager +tables_cache = LocalProxy(lambda: extensions.cache_manager.tables_cache) +talisman = extensions.talisman -# Flask-Compress -if conf.get("ENABLE_FLASK_COMPRESS"): - Compress(app) +__app_inited = False -talisman = Talisman() +def __fetch_or_init_app(): + global __app_inited + if not __app_inited: + create_app() + __app_inited = True -if app.config["TALISMAN_ENABLED"]: - talisman.init_app(app, **app.config["TALISMAN_CONFIG"]) + return current_app -# Hook that provides administrators a handle on the Flask APP -# after initialization -flask_app_mutator = app.config.get("FLASK_APP_MUTATOR") -if flask_app_mutator: - flask_app_mutator(app) -from superset import views # noqa +app = LocalProxy(__fetch_or_init_app) -# Registering sources -module_datasource_map = app.config.get("DEFAULT_MODULE_DS_MAP") -module_datasource_map.update(app.config.get("ADDITIONAL_MODULE_DS_MAP")) -ConnectorRegistry.register_sources(module_datasource_map) +created_app = create_app() diff --git a/superset/app.py b/superset/app.py new file mode 100644 index 0000000000000..ad6bf05b46962 --- /dev/null +++ b/superset/app.py @@ -0,0 +1,251 @@ +# 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. + +import logging +import os + +from flask_appbuilder import IndexView, expose +from flask_compress import Compress +from flask import Flask, redirect +from superset.extensions import ( + _event_logger, + APP_DIR, + appbuilder, + cache_manager, + db, + feature_flag_manager, + manifest_processor, + migrate, + results_backend_manager, + talisman +) +from flask_wtf import CSRFProtect + +from superset.connectors.connector_registry import ConnectorRegistry +from superset.security import SupersetSecurityManager +from superset.utils.core import pessimistic_connection_handling +from superset.utils.log import get_event_logger_from_cfg_value, DBEventLogger +import wtforms_json + + +logger = logging.getLogger(__name__) + + +def create_app(): + app = Flask(__name__) + + try: + # Allow user to override our config completely + config_module = os.environ.get("SUPERSET_CONFIG", "superset.config") + app.config.from_object(config_module) + + app_initializer = app.config.get("APP_INITIALIZER", SupersetAppInitializer)(app) + app_initializer.init_app() + + return app + + # Make sure that bootstrap errors ALWAYS get logged + except Exception as ex: + logger.exception("Failed to create app") + raise ex + + +class SupersetIndexView(IndexView): + @expose("/") + def index(self): + return redirect("/superset/welcome") + + +class SupersetAppInitializer: + def __init__(self, app: Flask) -> None: + super().__init__() + + self.flask_app = app + self.config = app.config + self.manifest = {} + + def pre_init(self) -> None: + """ + Called after all other init tasks are complete + """ + wtforms_json.init() + + if not os.path.exists(self.config["DATA_DIR"]): + os.makedirs(self.config["DATA_DIR"]) + + def post_init(self) -> None: + """ + Called before any other init tasks + """ + pass + + def init_views(self) -> None: + # TODO - This should iterate over all views and register them with FAB... + from superset import views # noqa + + def init_app_in_ctx(self) -> None: + """ + Runs init logic in the context of the app + """ + self.configure_fab() + + # self.configure_data_sources() + + # Hook that provides administrators a handle on the Flask APP + # after initialization + flask_app_mutator = self.config.get("FLASK_APP_MUTATOR") + if flask_app_mutator: + flask_app_mutator(self.flask_app) + + self.init_views() + + def init_app(self) -> None: + """ + Main entry point which will delegate to other methods in + order to fully init the app + """ + self.pre_init() + + self.setup_db() + + self.setup_event_logger() + + self.setup_bundle_manifest() + + self.register_blueprints() + + self.configure_wtf() + + self.configure_logging() + + self.configure_middlewares() + + self.configure_cache() + + with self.flask_app.app_context(): + self.init_app_in_ctx() + + self.post_init() + + def setup_event_logger(self): + _event_logger["event_logger"] = get_event_logger_from_cfg_value( + self.flask_app.config.get("EVENT_LOGGER", DBEventLogger()) + ) + + def configure_data_sources(self): + # Registering sources + module_datasource_map = self.config.get("DEFAULT_MODULE_DS_MAP") + module_datasource_map.update(self.config.get("ADDITIONAL_MODULE_DS_MAP")) + ConnectorRegistry.register_sources(module_datasource_map) + + def configure_cache(self): + cache_manager.init_app(self.flask_app) + results_backend_manager.init_app(self.flask_app) + + def configure_feature_flags(self): + feature_flag_manager.init_app(self.flask_app) + + def configure_fab(self): + if self.config.get("SILENCE_FAB"): + logging.getLogger("flask_appbuilder").setLevel(logging.ERROR) + + custom_sm = self.config.get("CUSTOM_SECURITY_MANAGER") or SupersetSecurityManager + if not issubclass(custom_sm, SupersetSecurityManager): + raise Exception( + """Your CUSTOM_SECURITY_MANAGER must now extend SupersetSecurityManager, + not FAB's security manager. + See [4565] in UPDATING.md""" + ) + + appbuilder.indexview = SupersetIndexView + appbuilder.base_template = "superset/base.html" + appbuilder.security_manager_class = custom_sm + appbuilder.update_perms = False + appbuilder.init_app(self.flask_app, db.session) + + def configure_middlewares(self): + if self.config.get("ENABLE_CORS"): + from flask_cors import CORS + + CORS(self.flask_app, **self.config.get("CORS_OPTIONS")) + + if self.config.get("ENABLE_PROXY_FIX"): + from werkzeug.middleware.proxy_fix import ProxyFix + + self.flask_app.wsgi_app = ProxyFix(self.flask_app.wsgi_app, **self.config.get("PROXY_FIX_CONFIG")) + + if self.config.get("ENABLE_CHUNK_ENCODING"): + + class ChunkedEncodingFix(object): + def __init__(self, app): + self.app = app + + def __call__(self, environ, start_response): + # Setting wsgi.input_terminated tells werkzeug.wsgi to ignore + # content-length and read the stream till the end. + if environ.get("HTTP_TRANSFER_ENCODING", "").lower() == u"chunked": + environ["wsgi.input_terminated"] = True + return self.app(environ, start_response) + + self.flask_app.wsgi_app = ChunkedEncodingFix(self.flask_app.wsgi_app) + + if self.config.get("UPLOAD_FOLDER"): + try: + os.makedirs(self.config.get("UPLOAD_FOLDER")) + except OSError: + pass + + for middleware in self.config.get("ADDITIONAL_MIDDLEWARE"): + self.flask_app.wsgi_app = middleware(self.flask_app.wsgi_app) + + # Flask-Compress + if self.config.get("ENABLE_FLASK_COMPRESS"): + Compress(self.flask_app) + + if self.config["TALISMAN_ENABLED"]: + talisman.init_app(self.flask_app, **self.config["TALISMAN_CONFIG"]) + + def configure_logging(self): + self.flask_app.config.get("LOGGING_CONFIGURATOR").configure_logging( + self.config, + self.flask_app.debug) + + def setup_db(self): + db.init_app(self.flask_app) + + with self.flask_app.app_context(): + pessimistic_connection_handling(db.engine) + + migrate.init_app(self.flask_app, db=db, directory=APP_DIR + "/migrations") + + def configure_wtf(self): + if self.config.get("WTF_CSRF_ENABLED"): + csrf = CSRFProtect(self.flask_app) + csrf_exempt_list = self.config.get("WTF_CSRF_EXEMPT_LIST", []) + for ex in csrf_exempt_list: + csrf.exempt(ex) + + def register_blueprints(self): + for bp in self.config.get("BLUEPRINTS"): + try: + logger.info("Registering blueprint: '{}'".format(bp.name)) + self.flask_app.register_blueprint(bp) + except Exception: + logger.exception("blueprint registration failed") + + def setup_bundle_manifest(self): + manifest_processor.init_app(self.flask_app) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 980e607772b33..5932c3478e5b5 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -49,10 +49,11 @@ from sqlalchemy.sql.expression import Label, Select, TextAsFrom import sqlparse -from superset import app, db, security_manager +from superset import app from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric from superset.db_engine_specs.base import TimestampExpression from superset.exceptions import DatabaseNotFound +from superset.extensions import db, security_manager from superset.jinja_context import get_template_processor from superset.models.annotations import Annotation from superset.models.core import Database diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 3b242ec8ed4de..eaa81c75b858f 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -28,8 +28,8 @@ from flask_babel import lazy_gettext as _ from wtforms.ext.sqlalchemy.fields import QuerySelectField -from superset import appbuilder, db, security_manager from superset.connectors.base.views import DatasourceModelView +from superset.extensions import appbuilder, db, security_manager from superset.utils import core as utils from superset.views.base import ( DatasourceFilter, diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 12911ad99f888..02c787d3a78df 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -37,7 +37,8 @@ import sqlparse from werkzeug.utils import secure_filename -from superset import app, db, sql_parse +from superset import sql_parse +from superset.extensions import db from superset.utils import core as utils if TYPE_CHECKING: @@ -52,11 +53,7 @@ class TimeGrain(NamedTuple): # pylint: disable=too-few-public-methods duration: Optional[str] -config = app.config - - QueryStatus = utils.QueryStatus -config = app.config builtin_time_grains: Dict[Optional[str], str] = { None: "Time Column", diff --git a/superset/db_engine_specs/hive.py b/superset/db_engine_specs/hive.py index a08edc22144f3..d0054f01f8f9d 100644 --- a/superset/db_engine_specs/hive.py +++ b/superset/db_engine_specs/hive.py @@ -22,6 +22,7 @@ from typing import Any, Dict, List, Optional, Tuple from urllib import parse +from flask import current_app from sqlalchemy import Column from sqlalchemy.engine import create_engine from sqlalchemy.engine.base import Engine @@ -30,13 +31,12 @@ from sqlalchemy.sql.expression import ColumnClause, Select from werkzeug.utils import secure_filename -from superset import app, conf from superset.db_engine_specs.base import BaseEngineSpec from superset.db_engine_specs.presto import PrestoEngineSpec from superset.utils import core as utils QueryStatus = utils.QueryStatus -config = app.config +conf = current_app.config tracking_url_trans = conf.get("TRACKING_URL_TRANSFORMER") hive_poll_interval = conf.get("HIVE_POLL_INTERVAL") @@ -63,6 +63,11 @@ class HiveEngineSpec(PrestoEngineSpec): r"reduce = (?P[0-9]+)%.*" ) + def __init__(self) -> None: + super().__init__() + self.tracking_url_trans = conf.get("TRACKING_URL_TRANSFORMER") + self.hive_poll_interval = conf.get("HIVE_POLL_INTERVAL") + @classmethod def patch(cls): from pyhive import hive # pylint: disable=no-name-in-module diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py index f2a2016935b01..ecf704ebe61a1 100644 --- a/superset/db_engine_specs/presto.py +++ b/superset/db_engine_specs/presto.py @@ -32,9 +32,9 @@ from sqlalchemy.engine.result import RowProxy from sqlalchemy.sql.expression import ColumnClause, Select -from superset import app, is_feature_enabled, security_manager from superset.db_engine_specs.base import BaseEngineSpec from superset.exceptions import SupersetTemplateException +from superset.extensions import feature_flag_manager, security_manager from superset.models.sql_types.presto_sql_types import type_map as presto_type_map from superset.sql_parse import ParsedQuery from superset.utils import core as utils @@ -44,7 +44,6 @@ from superset.models.core import Database # pylint: disable=unused-import QueryStatus = utils.QueryStatus -config = app.config # map between Presto types and Pandas pandas_dtype_map = { @@ -135,7 +134,7 @@ def get_table_names( cls, database: "Database", inspector: Inspector, schema: Optional[str] ) -> List[str]: tables = super().get_table_names(database, inspector, schema) - if not is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): + if not feature_flag_manager.is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): return tables views = set(cls.get_view_names(database, inspector, schema)) @@ -152,7 +151,7 @@ def get_view_names( and get_view_names() is not implemented in sqlalchemy_presto.py https://github.com/dropbox/PyHive/blob/e25fc8440a0686bbb7a5db5de7cb1a77bdb4167a/pyhive/sqlalchemy_presto.py """ - if not is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): + if not feature_flag_manager.is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): return [] if schema: @@ -336,7 +335,7 @@ def get_columns( for column in columns: try: # parse column if it is a row or array - if is_feature_enabled("PRESTO_EXPAND_DATA") and ( + if feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA") and ( "array" in column.Type or "row" in column.Type ): structural_column_index = len(result) @@ -423,7 +422,7 @@ def select_star( # pylint: disable=too-many-arguments """ cols = cols or [] presto_cols = cols - if is_feature_enabled("PRESTO_EXPAND_DATA") and show_cols: + if feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA") and show_cols: dot_regex = r"\.(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)" presto_cols = [ col for col in presto_cols if not re.search(dot_regex, col["name"]) @@ -577,7 +576,7 @@ def expand_data( # pylint: disable=too-many-locals :return: list of all columns(selected columns and their nested fields), expanded data set, listed of nested fields """ - if not is_feature_enabled("PRESTO_EXPAND_DATA"): + if not feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA"): return columns, data, [] # process each column, unnesting ARRAY types and diff --git a/superset/extensions.py b/superset/extensions.py new file mode 100644 index 0000000000000..ae2e059fce219 --- /dev/null +++ b/superset/extensions.py @@ -0,0 +1,43 @@ +# 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. + +import os + +from flask_appbuilder import AppBuilder, SQLA +from flask_migrate import Migrate +from flask_talisman import Talisman +from werkzeug.local import LocalProxy + +from superset.utils.feature_flag_manager import FeatureFlagManager + +from superset.utils.cache_manager import CacheManager +from superset.utils.manifest_processor import UIManifestProcessor +from superset.utils.results_backend_manager import ResultsBackendManager + +APP_DIR = os.path.dirname(__file__) + +appbuilder = AppBuilder(update_perms=False) +cache_manager = CacheManager() +db = SQLA() +_event_logger = {} +event_logger = LocalProxy(lambda: _event_logger.get("event_logger")) +feature_flag_manager = FeatureFlagManager() +manifest_processor = UIManifestProcessor(APP_DIR) +migrate = Migrate() +results_backend_manager = ResultsBackendManager() +security_manager = LocalProxy(lambda: appbuilder.sm) +talisman = Talisman() diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 2fdcb1424d72c..6460709c314ad 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -25,12 +25,10 @@ import uuid from dateutil.relativedelta import relativedelta -from flask import g, request +from flask import current_app, g, request from jinja2.sandbox import SandboxedEnvironment -from superset import app - -config = app.config +config = current_app.config BASE_CONTEXT = { "datetime": datetime, "random": random, diff --git a/superset/models/core.py b/superset/models/core.py index 200af496431b7..0c5cdb78fd880 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -24,7 +24,7 @@ import textwrap from typing import List -from flask import escape, g, Markup, request +from flask import current_app, escape, g, Markup, request from flask_appbuilder import Model from flask_appbuilder.models.decorators import renders from flask_appbuilder.security.sqla.models import User @@ -52,8 +52,9 @@ from sqlalchemy_utils import EncryptedType import sqlparse -from superset import app, db, db_engine_specs, is_feature_enabled, security_manager +from superset import db_engine_specs from superset.connectors.connector_registry import ConnectorRegistry +from superset.extensions import db, feature_flag_manager, security_manager from superset.legacy import update_time_range from superset.models.helpers import AuditMixinNullable, ImportMixin from superset.models.tags import ChartUpdater, DashboardUpdater, FavStarUpdater @@ -62,7 +63,7 @@ from superset.viz import viz_types from urllib import parse # noqa -config = app.config +config = current_app.config custom_password_store = config.get("SQLALCHEMY_CUSTOM_PASSWORD_STORE") stats_logger = config.get("STATS_LOGGER") log_query = config.get("QUERY_LOGGER") @@ -1310,7 +1311,7 @@ def user_roles(self): # events for updating tags -if is_feature_enabled("TAGGING_SYSTEM"): +if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) diff --git a/superset/models/schedules.py b/superset/models/schedules.py index dbcd56d83a27a..a525d57a612d1 100644 --- a/superset/models/schedules.py +++ b/superset/models/schedules.py @@ -24,7 +24,7 @@ from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.orm import relationship -from superset import security_manager +from superset.extensions import security_manager from superset.models.helpers import AuditMixinNullable, ImportMixin diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index 293932af3e3d5..1d7ba87133621 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -34,7 +34,7 @@ ) from sqlalchemy.orm import backref, relationship -from superset import security_manager +from superset.extensions import security_manager from superset.models.helpers import AuditMixinNullable, ExtraJSONMixin from superset.models.tags import QueryUpdater from superset.utils.core import QueryStatus, user_label diff --git a/superset/models/user_attributes.py b/superset/models/user_attributes.py index 2c69feb971acb..75ff2eebd7b3c 100644 --- a/superset/models/user_attributes.py +++ b/superset/models/user_attributes.py @@ -18,7 +18,7 @@ from sqlalchemy import Column, ForeignKey, Integer from sqlalchemy.orm import relationship -from superset import security_manager +from superset.extensions import security_manager from superset.models.helpers import AuditMixinNullable diff --git a/superset/utils/cache.py b/superset/utils/cache.py index 8fba7f888f1f2..2fe8a819048e9 100644 --- a/superset/utils/cache.py +++ b/superset/utils/cache.py @@ -15,9 +15,11 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=C,R,W -from flask import request +from typing import Optional -from superset import tables_cache +from flask import request, Flask +from flask_caching import Cache +from superset.extensions import cache_manager def view_cache_key(*unused_args, **unused_kwargs) -> str: @@ -43,7 +45,7 @@ def memoized_func(key=view_cache_key, attribute_in_key=None): """ def wrap(f): - if tables_cache: + if cache_manager.tables_cache: def wrapped_f(self, *args, **kwargs): if not kwargs.get("cache", True): @@ -55,11 +57,11 @@ def wrapped_f(self, *args, **kwargs): ) else: cache_key = key(*args, **kwargs) - o = tables_cache.get(cache_key) + o = cache_manager.tables_cache.get(cache_key) if not kwargs.get("force") and o is not None: return o o = f(self, *args, **kwargs) - tables_cache.set(cache_key, o, timeout=kwargs.get("cache_timeout")) + cache_manager.tables_cache.set(cache_key, o, timeout=kwargs.get("cache_timeout")) return o else: diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py new file mode 100644 index 0000000000000..81536f9df17ac --- /dev/null +++ b/superset/utils/cache_manager.py @@ -0,0 +1,40 @@ +from typing import Optional + +from flask import Flask +from flask_caching import Cache + + +class CacheManager: + def __init__(self) -> None: + super().__init__() + + self._tables_cache = None + self._cache = None + + def init_app(self, app): + self._cache = self._setup_cache(app, app.config.get("CACHE_CONFIG")) + self._tables_cache = self._setup_cache( + app, app.config.get("TABLE_NAMES_CACHE_CONFIG") + ) + + @staticmethod + def _setup_cache(app: Flask, cache_config) -> Optional[Cache]: + """Setup the flask-cache on a flask app""" + if cache_config: + if isinstance(cache_config, dict): + if cache_config.get("CACHE_TYPE") != "null": + return Cache(app, config=cache_config) + else: + # Accepts a custom cache initialization function, + # returning an object compatible with Flask-Caching API + return cache_config(app) + + return None + + @property + def tables_cache(self): + return self._tables_cache + + @property + def cache(self): + return self._cache diff --git a/superset/utils/core.py b/superset/utils/core.py index 88f8df1fc776d..2c4c174f662a7 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -791,20 +791,6 @@ def choicify(values): return [(v, v) for v in values] -def setup_cache(app: Flask, cache_config) -> Optional[Cache]: - """Setup the flask-cache on a flask app""" - if cache_config: - if isinstance(cache_config, dict): - if cache_config.get("CACHE_TYPE") != "null": - return Cache(app, config=cache_config) - else: - # Accepts a custom cache initialization function, - # returning an object compatible with Flask-Caching API - return cache_config(app) - - return None - - def zlib_compress(data): """ Compress things in a py2/3 safe fashion diff --git a/superset/utils/feature_flag_manager.py b/superset/utils/feature_flag_manager.py new file mode 100644 index 0000000000000..0bc849b65cbdd --- /dev/null +++ b/superset/utils/feature_flag_manager.py @@ -0,0 +1,40 @@ +# 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 copy import deepcopy + + +class FeatureFlagManager: + def __init__(self) -> None: + super().__init__() + self._get_feature_flags_func = None + self._feature_flags = {} + + def init_app(self, app): + self._get_feature_flags_func = app.config.get("GET_FEATURE_FLAGS_FUNC") + self._feature_flags = app.config.get("DEFAULT_FEATURE_FLAGS") or {} + self._feature_flags.update(app.config.get("FEATURE_FLAGS") or {}) + + def get_feature_flags(self): + if self._get_feature_flags_func: + return self._get_feature_flags_func(deepcopy(self._feature_flags)) + + return self._feature_flags + + def is_feature_enabled(self, feature): + """Utility function for checking whether a feature is turned on""" + return self.get_feature_flags().get(feature) diff --git a/superset/utils/manifest_processor.py b/superset/utils/manifest_processor.py new file mode 100644 index 0000000000000..73362709ba71a --- /dev/null +++ b/superset/utils/manifest_processor.py @@ -0,0 +1,69 @@ +# 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. + +import json + + +class UIManifestProcessor: + def __init__(self, app_dir: str) -> None: + super().__init__() + self.app = None + self.manifest = {} + self.manifest_file = f"{app_dir}/static/assets/dist/manifest.json" + + def init_app(self, app): + self.app = app + # Preload the cache + self.parse_manifest_json() + + @app.context_processor + def get_manifest(): + return dict( + loaded_chunks=set(), + get_unloaded_chunks=self.get_unloaded_chunks, + js_manifest=self.get_js_manifest_files, + css_manifest=self.get_css_manifest_files, + ) + + def parse_manifest_json(self): + try: + with open(self.manifest_file, "r") as f: + # the manifest includes non-entry files + # we only need entries in templates + full_manifest = json.load(f) + self.manifest = full_manifest.get("entrypoints", {}) + except Exception: + pass + + def get_js_manifest_files(self, filename): + if self.app.debug: + self.parse_manifest_json() + entry_files = self.manifest.get(filename, {}) + return entry_files.get("js", []) + + def get_css_manifest_files(self, filename): + if self.app.debug: + self.parse_manifest_json() + entry_files = self.manifest.get(filename, {}) + return entry_files.get("css", []) + + @staticmethod + def get_unloaded_chunks(files, loaded_chunks): + filtered_files = [f for f in files if f not in loaded_chunks] + for f in filtered_files: + loaded_chunks.add(f) + return filtered_files diff --git a/superset/utils/results_backend_manager.py b/superset/utils/results_backend_manager.py new file mode 100644 index 0000000000000..8317f753be77e --- /dev/null +++ b/superset/utils/results_backend_manager.py @@ -0,0 +1,35 @@ +# 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. + + +class ResultsBackendManager: + def __init__(self) -> None: + super().__init__() + self._results_backend = None + self._use_msgpack = False + + def init_app(self, app): + self._results_backend = app.config.get("RESULTS_BACKEND") + self._use_msgpack = app.config.get("RESULTS_BACKEND_USE_MSGPACK") + + @property + def results_backend(self): + return self.results_backend + + @property + def should_use_msgpack(self): + return self._use_msgpack diff --git a/superset/viz.py b/superset/viz.py index edd5b1caa4344..ac5c07e8f3685 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -46,8 +46,9 @@ import polyline import simplejson as json -from superset import app, cache, get_css_manifest_files +from flask import current_app from superset.exceptions import NullValueException, SpatialException +from superset.extensions import cache_manager, manifest_processor from superset.utils import core as utils from superset.utils.core import ( DTTM_ALIAS, @@ -57,7 +58,7 @@ ) -config = app.config +config = current_app.config stats_logger = config.get("STATS_LOGGER") relative_start = config.get("DEFAULT_RELATIVE_START_TIME", "today") relative_end = config.get("DEFAULT_RELATIVE_END_TIME", "today") @@ -394,8 +395,8 @@ def get_df_payload(self, query_obj=None, **kwargs): stacktrace = None df = None cached_dttm = datetime.utcnow().isoformat().split(".")[0] - if cache_key and cache and not self.force: - cache_value = cache.get(cache_key) + if cache_key and cache_manager.cache and not self.force: + cache_value = cache_manager.cache.get(cache_key) if cache_value: stats_logger.incr("loaded_from_cache") try: @@ -429,7 +430,7 @@ def get_df_payload(self, query_obj=None, **kwargs): if ( is_loaded and cache_key - and cache + and cache_manager.cache and self.status != utils.QueryStatus.FAILED ): try: @@ -445,13 +446,13 @@ def get_df_payload(self, query_obj=None, **kwargs): ) stats_logger.incr("set_cache_key") - cache.set(cache_key, cache_value, timeout=self.cache_timeout) + cache_manager.cache.set(cache_key, cache_value, timeout=self.cache_timeout) except Exception as e: # cache.set call can fail if the backend is down or if # the key is too large or whatever other reasons logging.warning("Could not cache key {}".format(cache_key)) logging.exception(e) - cache.delete(cache_key) + cache_manager.cache.delete(cache_key) return { "cache_key": self._any_cache_key, "cached_dttm": self._any_cached_dttm, @@ -735,7 +736,7 @@ def get_data(self, df): code = self.form_data.get("code", "") if markup_type == "markdown": code = markdown(code) - return dict(html=code, theme_css=get_css_manifest_files("theme")) + return dict(html=code, theme_css=manifest_processor.get_css_manifest_files("theme")) class SeparatorViz(MarkupViz): @@ -1292,7 +1293,7 @@ def get_data(self, df): fd = self.form_data # Late imports to avoid circular import issues from superset.models.core import Slice - from superset import db + from superset.extensions import db slice_ids1 = fd.get("line_charts") slices1 = db.session.query(Slice).filter(Slice.id.in_(slice_ids1)).all() @@ -2094,7 +2095,7 @@ def get_data(self, df): fd = self.form_data # Late imports to avoid circular import issues from superset.models.core import Slice - from superset import db + from superset.extensions import db slice_ids = fd.get("deck_slices") slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all() From 21e349440b6659cec4568573ab58ba7845100e84 Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 18 Oct 2019 13:46:01 -0700 Subject: [PATCH 02/42] Setting things back to master --- superset/connectors/sqla/models.py | 3 +-- superset/connectors/sqla/views.py | 2 +- superset/db_engine_specs/base.py | 7 +++++-- superset/db_engine_specs/hive.py | 9 ++------- superset/db_engine_specs/presto.py | 13 +++++++------ superset/jinja_context.py | 6 ++++-- superset/models/core.py | 9 ++++----- superset/models/schedules.py | 2 +- superset/models/sql_lab.py | 2 +- superset/models/user_attributes.py | 2 +- superset/viz.py | 21 ++++++++++----------- 11 files changed, 37 insertions(+), 39 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 5932c3478e5b5..980e607772b33 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -49,11 +49,10 @@ from sqlalchemy.sql.expression import Label, Select, TextAsFrom import sqlparse -from superset import app +from superset import app, db, security_manager from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric from superset.db_engine_specs.base import TimestampExpression from superset.exceptions import DatabaseNotFound -from superset.extensions import db, security_manager from superset.jinja_context import get_template_processor from superset.models.annotations import Annotation from superset.models.core import Database diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index eaa81c75b858f..3b242ec8ed4de 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -28,8 +28,8 @@ from flask_babel import lazy_gettext as _ from wtforms.ext.sqlalchemy.fields import QuerySelectField +from superset import appbuilder, db, security_manager from superset.connectors.base.views import DatasourceModelView -from superset.extensions import appbuilder, db, security_manager from superset.utils import core as utils from superset.views.base import ( DatasourceFilter, diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 02c787d3a78df..12911ad99f888 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -37,8 +37,7 @@ import sqlparse from werkzeug.utils import secure_filename -from superset import sql_parse -from superset.extensions import db +from superset import app, db, sql_parse from superset.utils import core as utils if TYPE_CHECKING: @@ -53,7 +52,11 @@ class TimeGrain(NamedTuple): # pylint: disable=too-few-public-methods duration: Optional[str] +config = app.config + + QueryStatus = utils.QueryStatus +config = app.config builtin_time_grains: Dict[Optional[str], str] = { None: "Time Column", diff --git a/superset/db_engine_specs/hive.py b/superset/db_engine_specs/hive.py index d0054f01f8f9d..a08edc22144f3 100644 --- a/superset/db_engine_specs/hive.py +++ b/superset/db_engine_specs/hive.py @@ -22,7 +22,6 @@ from typing import Any, Dict, List, Optional, Tuple from urllib import parse -from flask import current_app from sqlalchemy import Column from sqlalchemy.engine import create_engine from sqlalchemy.engine.base import Engine @@ -31,12 +30,13 @@ from sqlalchemy.sql.expression import ColumnClause, Select from werkzeug.utils import secure_filename +from superset import app, conf from superset.db_engine_specs.base import BaseEngineSpec from superset.db_engine_specs.presto import PrestoEngineSpec from superset.utils import core as utils QueryStatus = utils.QueryStatus -conf = current_app.config +config = app.config tracking_url_trans = conf.get("TRACKING_URL_TRANSFORMER") hive_poll_interval = conf.get("HIVE_POLL_INTERVAL") @@ -63,11 +63,6 @@ class HiveEngineSpec(PrestoEngineSpec): r"reduce = (?P[0-9]+)%.*" ) - def __init__(self) -> None: - super().__init__() - self.tracking_url_trans = conf.get("TRACKING_URL_TRANSFORMER") - self.hive_poll_interval = conf.get("HIVE_POLL_INTERVAL") - @classmethod def patch(cls): from pyhive import hive # pylint: disable=no-name-in-module diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py index ecf704ebe61a1..f2a2016935b01 100644 --- a/superset/db_engine_specs/presto.py +++ b/superset/db_engine_specs/presto.py @@ -32,9 +32,9 @@ from sqlalchemy.engine.result import RowProxy from sqlalchemy.sql.expression import ColumnClause, Select +from superset import app, is_feature_enabled, security_manager from superset.db_engine_specs.base import BaseEngineSpec from superset.exceptions import SupersetTemplateException -from superset.extensions import feature_flag_manager, security_manager from superset.models.sql_types.presto_sql_types import type_map as presto_type_map from superset.sql_parse import ParsedQuery from superset.utils import core as utils @@ -44,6 +44,7 @@ from superset.models.core import Database # pylint: disable=unused-import QueryStatus = utils.QueryStatus +config = app.config # map between Presto types and Pandas pandas_dtype_map = { @@ -134,7 +135,7 @@ def get_table_names( cls, database: "Database", inspector: Inspector, schema: Optional[str] ) -> List[str]: tables = super().get_table_names(database, inspector, schema) - if not feature_flag_manager.is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): + if not is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): return tables views = set(cls.get_view_names(database, inspector, schema)) @@ -151,7 +152,7 @@ def get_view_names( and get_view_names() is not implemented in sqlalchemy_presto.py https://github.com/dropbox/PyHive/blob/e25fc8440a0686bbb7a5db5de7cb1a77bdb4167a/pyhive/sqlalchemy_presto.py """ - if not feature_flag_manager.is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): + if not is_feature_enabled("PRESTO_SPLIT_VIEWS_FROM_TABLES"): return [] if schema: @@ -335,7 +336,7 @@ def get_columns( for column in columns: try: # parse column if it is a row or array - if feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA") and ( + if is_feature_enabled("PRESTO_EXPAND_DATA") and ( "array" in column.Type or "row" in column.Type ): structural_column_index = len(result) @@ -422,7 +423,7 @@ def select_star( # pylint: disable=too-many-arguments """ cols = cols or [] presto_cols = cols - if feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA") and show_cols: + if is_feature_enabled("PRESTO_EXPAND_DATA") and show_cols: dot_regex = r"\.(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)" presto_cols = [ col for col in presto_cols if not re.search(dot_regex, col["name"]) @@ -576,7 +577,7 @@ def expand_data( # pylint: disable=too-many-locals :return: list of all columns(selected columns and their nested fields), expanded data set, listed of nested fields """ - if not feature_flag_manager.is_feature_enabled("PRESTO_EXPAND_DATA"): + if not is_feature_enabled("PRESTO_EXPAND_DATA"): return columns, data, [] # process each column, unnesting ARRAY types and diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 6460709c314ad..2fdcb1424d72c 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -25,10 +25,12 @@ import uuid from dateutil.relativedelta import relativedelta -from flask import current_app, g, request +from flask import g, request from jinja2.sandbox import SandboxedEnvironment -config = current_app.config +from superset import app + +config = app.config BASE_CONTEXT = { "datetime": datetime, "random": random, diff --git a/superset/models/core.py b/superset/models/core.py index 0c5cdb78fd880..200af496431b7 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -24,7 +24,7 @@ import textwrap from typing import List -from flask import current_app, escape, g, Markup, request +from flask import escape, g, Markup, request from flask_appbuilder import Model from flask_appbuilder.models.decorators import renders from flask_appbuilder.security.sqla.models import User @@ -52,9 +52,8 @@ from sqlalchemy_utils import EncryptedType import sqlparse -from superset import db_engine_specs +from superset import app, db, db_engine_specs, is_feature_enabled, security_manager from superset.connectors.connector_registry import ConnectorRegistry -from superset.extensions import db, feature_flag_manager, security_manager from superset.legacy import update_time_range from superset.models.helpers import AuditMixinNullable, ImportMixin from superset.models.tags import ChartUpdater, DashboardUpdater, FavStarUpdater @@ -63,7 +62,7 @@ from superset.viz import viz_types from urllib import parse # noqa -config = current_app.config +config = app.config custom_password_store = config.get("SQLALCHEMY_CUSTOM_PASSWORD_STORE") stats_logger = config.get("STATS_LOGGER") log_query = config.get("QUERY_LOGGER") @@ -1311,7 +1310,7 @@ def user_roles(self): # events for updating tags -if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): +if is_feature_enabled("TAGGING_SYSTEM"): sqla.event.listen(Slice, "after_insert", ChartUpdater.after_insert) sqla.event.listen(Slice, "after_update", ChartUpdater.after_update) sqla.event.listen(Slice, "after_delete", ChartUpdater.after_delete) diff --git a/superset/models/schedules.py b/superset/models/schedules.py index a525d57a612d1..dbcd56d83a27a 100644 --- a/superset/models/schedules.py +++ b/superset/models/schedules.py @@ -24,7 +24,7 @@ from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.orm import relationship -from superset.extensions import security_manager +from superset import security_manager from superset.models.helpers import AuditMixinNullable, ImportMixin diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index 1d7ba87133621..293932af3e3d5 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -34,7 +34,7 @@ ) from sqlalchemy.orm import backref, relationship -from superset.extensions import security_manager +from superset import security_manager from superset.models.helpers import AuditMixinNullable, ExtraJSONMixin from superset.models.tags import QueryUpdater from superset.utils.core import QueryStatus, user_label diff --git a/superset/models/user_attributes.py b/superset/models/user_attributes.py index 75ff2eebd7b3c..2c69feb971acb 100644 --- a/superset/models/user_attributes.py +++ b/superset/models/user_attributes.py @@ -18,7 +18,7 @@ from sqlalchemy import Column, ForeignKey, Integer from sqlalchemy.orm import relationship -from superset.extensions import security_manager +from superset import security_manager from superset.models.helpers import AuditMixinNullable diff --git a/superset/viz.py b/superset/viz.py index ac5c07e8f3685..edd5b1caa4344 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -46,9 +46,8 @@ import polyline import simplejson as json -from flask import current_app +from superset import app, cache, get_css_manifest_files from superset.exceptions import NullValueException, SpatialException -from superset.extensions import cache_manager, manifest_processor from superset.utils import core as utils from superset.utils.core import ( DTTM_ALIAS, @@ -58,7 +57,7 @@ ) -config = current_app.config +config = app.config stats_logger = config.get("STATS_LOGGER") relative_start = config.get("DEFAULT_RELATIVE_START_TIME", "today") relative_end = config.get("DEFAULT_RELATIVE_END_TIME", "today") @@ -395,8 +394,8 @@ def get_df_payload(self, query_obj=None, **kwargs): stacktrace = None df = None cached_dttm = datetime.utcnow().isoformat().split(".")[0] - if cache_key and cache_manager.cache and not self.force: - cache_value = cache_manager.cache.get(cache_key) + if cache_key and cache and not self.force: + cache_value = cache.get(cache_key) if cache_value: stats_logger.incr("loaded_from_cache") try: @@ -430,7 +429,7 @@ def get_df_payload(self, query_obj=None, **kwargs): if ( is_loaded and cache_key - and cache_manager.cache + and cache and self.status != utils.QueryStatus.FAILED ): try: @@ -446,13 +445,13 @@ def get_df_payload(self, query_obj=None, **kwargs): ) stats_logger.incr("set_cache_key") - cache_manager.cache.set(cache_key, cache_value, timeout=self.cache_timeout) + cache.set(cache_key, cache_value, timeout=self.cache_timeout) except Exception as e: # cache.set call can fail if the backend is down or if # the key is too large or whatever other reasons logging.warning("Could not cache key {}".format(cache_key)) logging.exception(e) - cache_manager.cache.delete(cache_key) + cache.delete(cache_key) return { "cache_key": self._any_cache_key, "cached_dttm": self._any_cached_dttm, @@ -736,7 +735,7 @@ def get_data(self, df): code = self.form_data.get("code", "") if markup_type == "markdown": code = markdown(code) - return dict(html=code, theme_css=manifest_processor.get_css_manifest_files("theme")) + return dict(html=code, theme_css=get_css_manifest_files("theme")) class SeparatorViz(MarkupViz): @@ -1293,7 +1292,7 @@ def get_data(self, df): fd = self.form_data # Late imports to avoid circular import issues from superset.models.core import Slice - from superset.extensions import db + from superset import db slice_ids1 = fd.get("line_charts") slices1 = db.session.query(Slice).filter(Slice.id.in_(slice_ids1)).all() @@ -2095,7 +2094,7 @@ def get_data(self, df): fd = self.form_data # Late imports to avoid circular import issues from superset.models.core import Slice - from superset.extensions import db + from superset import db slice_ids = fd.get("deck_slices") slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all() From be37d243fbf8ca5877b142ccd2cde92783f88d0f Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 18 Oct 2019 15:39:31 -0700 Subject: [PATCH 03/42] Working with new FLASK_APP --- superset/__init__.py | 57 ++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 9c1447fe89ba4..2aa623b72937b 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -15,41 +15,36 @@ # specific language governing permissions and limitations # under the License. """Package's main module!""" -from flask import current_app +from flask import current_app as flask_current_app from werkzeug.local import LocalProxy from superset.app import create_app from superset.connectors.connector_registry import ConnectorRegistry -from superset.extensions import appbuilder +from superset.extensions import ( + appbuilder as ab, + cache_manager as ext_cache_manager, + db as ext_db, + event_logger as ext_event_logger, + feature_flag_manager as ext_feature_flag_manager, + manifest_processor as ext_manifest_processor, + results_backend_manager as ext_results_backend_manager, + security_manager as ext_security_manager, + talisman as ext_talisman +) from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value -appbuilder = extensions.appbuilder -cache = LocalProxy(lambda: extensions.cache_manager.cache) -conf = LocalProxy(lambda: current_app.config) -db = extensions.db -event_logger = extensions.event_logger -get_feature_flags = extensions.feature_flag_manager.get_feature_flags -is_feature_enabled = extensions.feature_flag_manager.is_feature_enabled -results_backend = LocalProxy(lambda: extensions.results_backend_manager.results_backend) -results_backend_use_msgpack = LocalProxy(lambda: extensions.results_backend_manager.should_use_msgpack) -security_manager = extensions.security_manager -tables_cache = LocalProxy(lambda: extensions.cache_manager.tables_cache) -talisman = extensions.talisman - - -__app_inited = False - - -def __fetch_or_init_app(): - global __app_inited - if not __app_inited: - create_app() - __app_inited = True - - return current_app - - -app = LocalProxy(__fetch_or_init_app) - -created_app = create_app() +app = flask_current_app +appbuilder = ab +cache = LocalProxy(lambda: ext_cache_manager.cache) +conf = LocalProxy(lambda: flask_current_app.config) +db = ext_db +event_logger = ext_event_logger +get_feature_flags = ext_feature_flag_manager.get_feature_flags +get_css_manifest_files = ext_manifest_processor.get_css_manifest_files +is_feature_enabled = ext_feature_flag_manager.is_feature_enabled +results_backend = LocalProxy(lambda: ext_results_backend_manager.results_backend) +results_backend_use_msgpack = LocalProxy(lambda: ext_results_backend_manager.should_use_msgpack) +security_manager = ext_security_manager +tables_cache = LocalProxy(lambda: ext_cache_manager.tables_cache) +talisman = ext_talisman From 259a30783a4e5ff9cd6a3bdc786966806f4fd9e2 Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 21 Oct 2019 09:16:33 -0700 Subject: [PATCH 04/42] Still need to refactor Celery --- superset/__init__.py | 2 +- superset/app.py | 2 +- superset/extensions.py | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 2aa623b72937b..5c80c351e17f3 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -34,7 +34,7 @@ from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value -app = flask_current_app +app = create_app() appbuilder = ab cache = LocalProxy(lambda: ext_cache_manager.cache) conf = LocalProxy(lambda: flask_current_app.config) diff --git a/superset/app.py b/superset/app.py index ad6bf05b46962..de488c17730cf 100644 --- a/superset/app.py +++ b/superset/app.py @@ -46,7 +46,7 @@ def create_app(): - app = Flask(__name__) + try: # Allow user to override our config completely diff --git a/superset/extensions.py b/superset/extensions.py index ae2e059fce219..59e8a7af8114f 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -17,6 +17,7 @@ import os +from flask import Flask from flask_appbuilder import AppBuilder, SQLA from flask_migrate import Migrate from flask_talisman import Talisman @@ -30,6 +31,7 @@ APP_DIR = os.path.dirname(__file__) +app = Flask(__name__) appbuilder = AppBuilder(update_perms=False) cache_manager = CacheManager() db = SQLA() From 89bba875a78ab4172629c2a7c7436c43daf65cbc Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 28 Oct 2019 09:55:55 -0700 Subject: [PATCH 05/42] CLI mostly working --- superset/__init__.py | 2 +- superset/app.py | 2 +- superset/bin/superset | 2 +- superset/cli.py | 4 ---- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 5c80c351e17f3..2aa623b72937b 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -34,7 +34,7 @@ from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value -app = create_app() +app = flask_current_app appbuilder = ab cache = LocalProxy(lambda: ext_cache_manager.cache) conf = LocalProxy(lambda: flask_current_app.config) diff --git a/superset/app.py b/superset/app.py index de488c17730cf..ad6bf05b46962 100644 --- a/superset/app.py +++ b/superset/app.py @@ -46,7 +46,7 @@ def create_app(): - + app = Flask(__name__) try: # Allow user to override our config completely diff --git a/superset/bin/superset b/superset/bin/superset index 0617335e99f29..d8c345c7cb71c 100755 --- a/superset/bin/superset +++ b/superset/bin/superset @@ -18,7 +18,7 @@ import click from flask.cli import FlaskGroup -from superset.cli import create_app +from superset.app import create_app @click.group(cls=FlaskGroup, create_app=create_app) diff --git a/superset/cli.py b/superset/cli.py index eea034a039d30..32a8bab946354 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -36,10 +36,6 @@ celery_app = utils.get_celery_app(config) -def create_app(script_info=None): - return app - - @app.shell_context_processor def make_shell_context(): return dict(app=app, db=db) From fb9cf33adc86833dd708e8609639a4e9ca633d8c Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 28 Oct 2019 13:10:54 -0700 Subject: [PATCH 06/42] Working on unit tests --- superset/app.py | 10 ++++++++-- superset/extensions.py | 3 ++- superset/sql_lab.py | 2 +- superset/tasks/cache.py | 2 +- superset/tasks/celery_app.py | 17 +++++++++++------ superset/tasks/schedules.py | 2 +- superset/utils/core.py | 13 ------------- tests/__init__.py | 7 +++++++ tests/base_tests.py | 4 ++-- 9 files changed, 33 insertions(+), 27 deletions(-) diff --git a/superset/app.py b/superset/app.py index ad6bf05b46962..6fec53fed42ed 100644 --- a/superset/app.py +++ b/superset/app.py @@ -31,8 +31,8 @@ manifest_processor, migrate, results_backend_manager, - talisman -) + talisman, + celery_app) from flask_wtf import CSRFProtect from superset.connectors.connector_registry import ConnectorRegistry @@ -93,6 +93,10 @@ def post_init(self) -> None: """ pass + def configure_celery(self) -> None: + celery_app.config_from_object(self.config.get("CELERY_CONFIG")) + celery_app.set_default() + def init_views(self) -> None: # TODO - This should iterate over all views and register them with FAB... from superset import views # noqa @@ -122,6 +126,8 @@ def init_app(self) -> None: self.setup_db() + self.configure_celery() + self.setup_event_logger() self.setup_bundle_manifest() diff --git a/superset/extensions.py b/superset/extensions.py index 59e8a7af8114f..b27599c5f85a4 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -17,6 +17,7 @@ import os +import celery from flask import Flask from flask_appbuilder import AppBuilder, SQLA from flask_migrate import Migrate @@ -31,9 +32,9 @@ APP_DIR = os.path.dirname(__file__) -app = Flask(__name__) appbuilder = AppBuilder(update_perms=False) cache_manager = CacheManager() +celery_app = celery.Celery() db = SQLA() _event_logger = {} event_logger = LocalProxy(lambda: _event_logger.get("event_logger")) diff --git a/superset/sql_lab.py b/superset/sql_lab.py index 9050f06d95bbd..40c16d7114c14 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -44,7 +44,7 @@ from superset.db_engine_specs import BaseEngineSpec from superset.models.sql_lab import Query from superset.sql_parse import ParsedQuery -from superset.tasks.celery_app import app as celery_app +from superset.extensions import celery_app from superset.utils.core import json_iso_dttm_ser, QueryStatus, sources, zlib_compress from superset.utils.dates import now_as_float from superset.utils.decorators import stats_timing diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index 22da4f9a48944..94bb7307c2133 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -27,7 +27,7 @@ from superset import app, db from superset.models.core import Dashboard, Log, Slice from superset.models.tags import Tag, TaggedObject -from superset.tasks.celery_app import app as celery_app +from superset.extensions import celery_app from superset.utils.core import parse_human_datetime logger = get_task_logger(__name__) diff --git a/superset/tasks/celery_app.py b/superset/tasks/celery_app.py index 1c0305a5b0d6c..fa172276391ff 100644 --- a/superset/tasks/celery_app.py +++ b/superset/tasks/celery_app.py @@ -16,12 +16,17 @@ # under the License. # pylint: disable=C,R,W -"""Utility functions used across Superset""" +""" +This is the main entrypoint used by Celery workers. As such, +it needs to call create_app() in order to initialize things properly +""" # Superset framework imports -from superset import app -from superset.utils.core import get_celery_app +from superset import create_app +from superset.extensions import celery_app -# Globals -config = app.config -app = get_celery_app(config) +# Init the Flask app / configure everything +create_app() + +# Export the celery app globally for Celery (as run on the cmd line) to find +app = celery_app diff --git a/superset/tasks/schedules.py b/superset/tasks/schedules.py index 6bda5b7ba2b6c..d06dd84d4d693 100644 --- a/superset/tasks/schedules.py +++ b/superset/tasks/schedules.py @@ -45,7 +45,7 @@ ScheduleType, SliceEmailReportFormat, ) -from superset.tasks.celery_app import app as celery_app +from superset.extensions import celery_app from superset.utils.core import get_email_address_list, send_email_smtp # Globals diff --git a/superset/utils/core.py b/superset/utils/core.py index 437a2e985325f..0573fd0876cbe 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -817,19 +817,6 @@ def zlib_decompress(blob: bytes, decode: Optional[bool] = True) -> Union[bytes, return decompressed.decode("utf-8") if decode else decompressed -_celery_app = None - - -def get_celery_app(config): - global _celery_app - if _celery_app: - return _celery_app - _celery_app = celery.Celery() - _celery_app.config_from_object(config.get("CELERY_CONFIG")) - _celery_app.set_default() - return _celery_app - - def to_adhoc(filt, expressionType="SIMPLE", clause="where"): result = { "clause": clause.upper(), diff --git a/tests/__init__.py b/tests/__init__.py index 13a83393a9124..2395a3abe89d6 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -14,3 +14,10 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. + +""" +TODO: Clean this up! The current pattern of accessing app props on package init means + that we need to ensure the creation of our Flask app BEFORE any tests load +""" +from superset.app import create_app +create_app().app_context().push() diff --git a/tests/base_tests.py b/tests/base_tests.py index 7fb1c99c26f38..4a5dc836ee54b 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -246,11 +246,11 @@ def validate_sql( raise Exception("validate_sql failed") return resp - @patch.dict("superset._feature_flags", {"FOO": True}, clear=True) + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {"FOO": True}, clear=True) def test_existing_feature_flags(self): self.assertTrue(is_feature_enabled("FOO")) - @patch.dict("superset._feature_flags", {}, clear=True) + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {}, clear=True) def test_nonexistent_feature_flags(self): self.assertFalse(is_feature_enabled("FOO")) From 01aafb5ae2ddf1bd8bc9dd2b2518f4d5ada64dd2 Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 28 Oct 2019 14:54:11 -0700 Subject: [PATCH 07/42] Moving cli stuff around a bit --- superset/app.py | 1 + superset/bin/superset | 13 ++------ superset/cli.py | 59 ++++++++++++++++++++++--------------- tests/base_tests.py | 16 ++-------- tests/feature_flag_tests.py | 17 +++++++++++ 5 files changed, 57 insertions(+), 49 deletions(-) create mode 100644 tests/feature_flag_tests.py diff --git a/superset/app.py b/superset/app.py index 6fec53fed42ed..738b4ab13797b 100644 --- a/superset/app.py +++ b/superset/app.py @@ -106,6 +106,7 @@ def init_app_in_ctx(self) -> None: Runs init logic in the context of the app """ self.configure_fab() + self.configure_data_sources() # self.configure_data_sources() diff --git a/superset/bin/superset b/superset/bin/superset index d8c345c7cb71c..0f5239726a2cc 100755 --- a/superset/bin/superset +++ b/superset/bin/superset @@ -15,17 +15,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import click -from flask.cli import FlaskGroup - -from superset.app import create_app - - -@click.group(cls=FlaskGroup, create_app=create_app) -def cli(): - """This is a management script for the Superset application.""" - pass +from superset.cli import superset if __name__ == '__main__': - cli() + superset() diff --git a/superset/cli.py b/superset/cli.py index 32a8bab946354..675fb3a8e5872 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -25,23 +25,34 @@ import yaml from colorama import Fore, Style from flask import g +from flask.cli import FlaskGroup, with_appcontext from flask_appbuilder import Model from pathlib2 import Path -from superset import app, appbuilder, db, examples, security_manager -from superset.common.tags import add_favorites, add_owners, add_types -from superset.utils import core as utils, dashboard_import_export, dict_import_export +# from superset import app, appbuilder, db, examples, security_manager, create_app +# from superset.common.tags import add_favorites, add_owners, add_types +# from superset.extensions import celery_app +# from superset.utils import core as utils, dashboard_import_export, dict_import_export -config = app.config -celery_app = utils.get_celery_app(config) +#config = app.config +from superset import app, appbuilder, security_manager +from superset.app import create_app +from superset.extensions import celery_app, db +from superset.utils import core as utils -@app.shell_context_processor -def make_shell_context(): - return dict(app=app, db=db) +@click.group(cls=FlaskGroup, create_app=create_app) +@with_appcontext +def superset(): + """This is a management script for the Superset application.""" + @app.shell_context_processor + def make_shell_context(): + return dict(app=app, db=db) -@app.cli.command() + +@superset.command() +@with_appcontext def init(): """Inits the Superset application""" utils.get_example_database() @@ -49,7 +60,7 @@ def init(): security_manager.sync_role_definitions() -@app.cli.command() +@superset.command() @click.option("--verbose", "-v", is_flag=True, help="Show extra information") def version(verbose): """Prints the current version number""" @@ -125,7 +136,7 @@ def load_examples_run(load_test_data, only_metadata=False, force=False): examples.load_tabbed_dashboard(only_metadata) -@app.cli.command() +@superset.command() @click.option("--load-test-data", "-t", is_flag=True, help="Load additional test data") @click.option( "--only-metadata", "-m", is_flag=True, help="Only load metadata, skip actual data" @@ -138,7 +149,7 @@ def load_examples(load_test_data, only_metadata=False, force=False): load_examples_run(load_test_data, only_metadata, force) -@app.cli.command() +@superset.command() @click.option("--database_name", "-d", help="Database name to change") @click.option("--uri", "-u", help="Database URI to change") def set_database_uri(database_name, uri): @@ -146,7 +157,7 @@ def set_database_uri(database_name, uri): utils.get_or_create_db(database_name, uri) -@app.cli.command() +@superset.command() @click.option( "--datasource", "-d", @@ -176,7 +187,7 @@ def refresh_druid(datasource, merge): session.commit() -@app.cli.command() +@superset.command() @click.option( "--path", "-p", @@ -218,7 +229,7 @@ def import_dashboards(path, recursive, username): logging.error(e) -@app.cli.command() +@superset.command() @click.option( "--dashboard-file", "-f", default=None, help="Specify the the file to export to" ) @@ -236,7 +247,7 @@ def export_dashboards(print_stdout, dashboard_file): data_stream.write(data) -@app.cli.command() +@superset.command() @click.option( "--path", "-p", @@ -284,7 +295,7 @@ def import_datasources(path, sync, recursive): logging.error(e) -@app.cli.command() +@superset.command() @click.option( "--datasource-file", "-f", default=None, help="Specify the the file to export to" ) @@ -323,7 +334,7 @@ def export_datasources( yaml.safe_dump(data, data_stream, default_flow_style=False) -@app.cli.command() +@superset.command() @click.option( "--back-references", "-b", @@ -337,7 +348,7 @@ def export_datasource_schema(back_references): yaml.safe_dump(data, stdout, default_flow_style=False) -@app.cli.command() +@superset.command() def update_datasources_cache(): """Refresh sqllab datasources cache""" from superset.models.core import Database @@ -356,7 +367,7 @@ def update_datasources_cache(): print("{}".format(str(e))) -@app.cli.command() +@superset.command() @click.option( "--workers", "-w", type=int, help="Number of celery server workers to fire up" ) @@ -377,7 +388,7 @@ def worker(workers): worker.start() -@app.cli.command() +@superset.command() @click.option( "-p", "--port", default="5555", help="Port on which to start the Flower process" ) @@ -407,7 +418,7 @@ def flower(port, address): Popen(cmd, shell=True).wait() -@app.cli.command() +@superset.command() def load_test_users(): """ Loads admin, alpha, and gamma user for testing purposes @@ -424,7 +435,7 @@ def load_test_users_run(): Syncs permissions for those users/roles """ - if config.get("TESTING"): + if app.config.get("TESTING"): sm = security_manager @@ -461,7 +472,7 @@ def load_test_users_run(): sm.get_session.commit() -@app.cli.command() +@superset.command() def sync_tags(): """Rebuilds special tags (owner, type, favorited by).""" # pylint: disable=no-member diff --git a/tests/base_tests.py b/tests/base_tests.py index 4a5dc836ee54b..ed33edb488f5b 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -18,12 +18,12 @@ import imp import json import unittest -from unittest.mock import Mock, patch +from unittest.mock import Mock import pandas as pd from flask_appbuilder.security.sqla import models as ab_models -from superset import app, db, is_feature_enabled, security_manager +from superset import app, db, security_manager from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models @@ -246,18 +246,6 @@ def validate_sql( raise Exception("validate_sql failed") return resp - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {"FOO": True}, clear=True) - def test_existing_feature_flags(self): - self.assertTrue(is_feature_enabled("FOO")) - - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {}, clear=True) - def test_nonexistent_feature_flags(self): - self.assertFalse(is_feature_enabled("FOO")) - - def test_feature_flags(self): - self.assertEqual(is_feature_enabled("foo"), "bar") - self.assertEqual(is_feature_enabled("super"), "set") - def get_dash_by_slug(self, dash_slug): sesh = db.session() return sesh.query(models.Dashboard).filter_by(slug=dash_slug).first() diff --git a/tests/feature_flag_tests.py b/tests/feature_flag_tests.py new file mode 100644 index 0000000000000..f698016637d9f --- /dev/null +++ b/tests/feature_flag_tests.py @@ -0,0 +1,17 @@ +from superset import is_feature_enabled +from tests.base_tests import SupersetTestCase +from unittest.mock import patch + + +class FeatureFlagTests(SupersetTestCase): + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {"FOO": True}, clear=True) + def test_existing_feature_flags(self): + self.assertTrue(is_feature_enabled("FOO")) + + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {}, clear=True) + def test_nonexistent_feature_flags(self): + self.assertFalse(is_feature_enabled("FOO")) + + def test_feature_flags(self): + self.assertEqual(is_feature_enabled("foo"), "bar") + self.assertEqual(is_feature_enabled("super"), "set") From db25d4d3be1b46c1e75bafdcdb6b226490b1612b Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 6 Nov 2019 10:56:43 -0800 Subject: [PATCH 08/42] Removing get in config --- superset/app.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/superset/app.py b/superset/app.py index 738b4ab13797b..7eeff58dba9b5 100644 --- a/superset/app.py +++ b/superset/app.py @@ -94,7 +94,7 @@ def post_init(self) -> None: pass def configure_celery(self) -> None: - celery_app.config_from_object(self.config.get("CELERY_CONFIG")) + celery_app.config_from_object(self.config["CELERY_CONFIG"]) celery_app.set_default() def init_views(self) -> None: @@ -112,7 +112,7 @@ def init_app_in_ctx(self) -> None: # Hook that provides administrators a handle on the Flask APP # after initialization - flask_app_mutator = self.config.get("FLASK_APP_MUTATOR") + flask_app_mutator = self.config["FLASK_APP_MUTATOR"] if flask_app_mutator: flask_app_mutator(self.flask_app) @@ -155,8 +155,8 @@ def setup_event_logger(self): def configure_data_sources(self): # Registering sources - module_datasource_map = self.config.get("DEFAULT_MODULE_DS_MAP") - module_datasource_map.update(self.config.get("ADDITIONAL_MODULE_DS_MAP")) + module_datasource_map = self.config["DEFAULT_MODULE_DS_MAP"] + module_datasource_map.update(self.config["ADDITIONAL_MODULE_DS_MAP"]) ConnectorRegistry.register_sources(module_datasource_map) def configure_cache(self): @@ -167,10 +167,10 @@ def configure_feature_flags(self): feature_flag_manager.init_app(self.flask_app) def configure_fab(self): - if self.config.get("SILENCE_FAB"): + if self.config["SILENCE_FAB"]: logging.getLogger("flask_appbuilder").setLevel(logging.ERROR) - custom_sm = self.config.get("CUSTOM_SECURITY_MANAGER") or SupersetSecurityManager + custom_sm = self.config["CUSTOM_SECURITY_MANAGER"] or SupersetSecurityManager if not issubclass(custom_sm, SupersetSecurityManager): raise Exception( """Your CUSTOM_SECURITY_MANAGER must now extend SupersetSecurityManager, @@ -185,17 +185,17 @@ def configure_fab(self): appbuilder.init_app(self.flask_app, db.session) def configure_middlewares(self): - if self.config.get("ENABLE_CORS"): + if self.config["ENABLE_CORS"]: from flask_cors import CORS - CORS(self.flask_app, **self.config.get("CORS_OPTIONS")) + CORS(self.flask_app, **self.config["CORS_OPTIONS"]) - if self.config.get("ENABLE_PROXY_FIX"): + if self.config["ENABLE_PROXY_FIX"]: from werkzeug.middleware.proxy_fix import ProxyFix - self.flask_app.wsgi_app = ProxyFix(self.flask_app.wsgi_app, **self.config.get("PROXY_FIX_CONFIG")) + self.flask_app.wsgi_app = ProxyFix(self.flask_app.wsgi_app, **self.config["PROXY_FIX_CONFIG"]) - if self.config.get("ENABLE_CHUNK_ENCODING"): + if self.config["ENABLE_CHUNK_ENCODING"]: class ChunkedEncodingFix(object): def __init__(self, app): @@ -210,17 +210,17 @@ def __call__(self, environ, start_response): self.flask_app.wsgi_app = ChunkedEncodingFix(self.flask_app.wsgi_app) - if self.config.get("UPLOAD_FOLDER"): + if self.config["UPLOAD_FOLDER"]: try: - os.makedirs(self.config.get("UPLOAD_FOLDER")) + os.makedirs(self.config["UPLOAD_FOLDER"]) except OSError: pass - for middleware in self.config.get("ADDITIONAL_MIDDLEWARE"): + for middleware in self.config["ADDITIONAL_MIDDLEWARE"]: self.flask_app.wsgi_app = middleware(self.flask_app.wsgi_app) # Flask-Compress - if self.config.get("ENABLE_FLASK_COMPRESS"): + if self.config["ENABLE_FLASK_COMPRESS"]: Compress(self.flask_app) if self.config["TALISMAN_ENABLED"]: @@ -240,14 +240,14 @@ def setup_db(self): migrate.init_app(self.flask_app, db=db, directory=APP_DIR + "/migrations") def configure_wtf(self): - if self.config.get("WTF_CSRF_ENABLED"): + if self.config["WTF_CSRF_ENABLED"]: csrf = CSRFProtect(self.flask_app) csrf_exempt_list = self.config.get("WTF_CSRF_EXEMPT_LIST", []) for ex in csrf_exempt_list: csrf.exempt(ex) def register_blueprints(self): - for bp in self.config.get("BLUEPRINTS"): + for bp in self.config["BLUEPRINTS"]: try: logger.info("Registering blueprint: '{}'".format(bp.name)) self.flask_app.register_blueprint(bp) From 0ab9f0a783b0341c32315e036df358d7926a76a9 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 6 Nov 2019 13:41:58 -0800 Subject: [PATCH 09/42] Defaulting test config --- tests/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/__init__.py b/tests/__init__.py index 2395a3abe89d6..b9e6d9d8959d0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -19,5 +19,8 @@ TODO: Clean this up! The current pattern of accessing app props on package init means that we need to ensure the creation of our Flask app BEFORE any tests load """ +from os import environ +environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") + from superset.app import create_app create_app().app_context().push() From 921c285892441a9f8c565f318e5056c5aa5b18b2 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 7 Nov 2019 15:46:17 -0800 Subject: [PATCH 10/42] Adding flask-testing --- requirements-dev.txt | 1 + setup.py | 1 + tests/__init__.py | 4 +++- tests/base_tests.py | 15 +++++++++++---- tests/core_tests.py | 9 +++------ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 63116507b5533..9a53de611119e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -17,6 +17,7 @@ black==19.3b0 coverage==4.5.3 flask-cors==3.0.7 +flask-testing==0.7.1 ipdb==0.12 isort==4.3.21 mypy==0.670 diff --git a/setup.py b/setup.py index b2d9cea8d4c94..bffd649f7f159 100644 --- a/setup.py +++ b/setup.py @@ -110,6 +110,7 @@ def get_git_sha(): extras_require={ "bigquery": ["pybigquery>=0.4.10", "pandas_gbq>=0.10.0"], "cors": ["flask-cors>=2.0.0"], + "flask-testing": ["Flask-Testing==0.7.1"], "gsheets": ["gsheetsdb>=0.1.9"], "hive": ["pyhive[hive]>=0.6.1", "tableschema", "thrift>=0.11.0, <1.0.0"], "mysql": ["mysqlclient==1.4.2.post1"], diff --git a/tests/__init__.py b/tests/__init__.py index b9e6d9d8959d0..17826c80897b0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -23,4 +23,6 @@ environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") from superset.app import create_app -create_app().app_context().push() +app = create_app() +# ctx = app.test_request_context() +# ctx.push() diff --git a/tests/base_tests.py b/tests/base_tests.py index 872da6fbc6c78..217ae64fbf368 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -22,22 +22,29 @@ import pandas as pd from flask_appbuilder.security.sqla import models as ab_models +from flask_testing import TestCase from superset import app, db, security_manager +from superset.app import create_app from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models from superset.models.core import Database from superset.utils.core import get_example_database -BASE_DIR = app.config["BASE_DIR"] - -class SupersetTestCase(unittest.TestCase): +class SupersetTestCase(TestCase): def __init__(self, *args, **kwargs): super(SupersetTestCase, self).__init__(*args, **kwargs) - self.client = app.test_client() self.maxDiff = None + self.BASE_DIR = "" + + def create_app(self): + from tests import app + return app + + def setUp(self) -> None: + self.BASE_DIR = app.config["BASE_DIR"] @classmethod def create_druid_test_objects(cls): diff --git a/tests/core_tests.py b/tests/core_tests.py index e2cf010b6ae88..9b546d772b3eb 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -50,16 +50,13 @@ class CoreTests(SupersetTestCase): def __init__(self, *args, **kwargs): super(CoreTests, self).__init__(*args, **kwargs) - @classmethod - def setUpClass(cls): - cls.table_ids = { - tbl.table_name: tbl.id for tbl in (db.session.query(SqlaTable).all()) - } - def setUp(self): db.session.query(Query).delete() db.session.query(models.DatasourceAccessRequest).delete() db.session.query(models.Log).delete() + self.table_ids = { + tbl.table_name: tbl.id for tbl in (db.session.query(SqlaTable).all()) + } def tearDown(self): db.session.query(Query).delete() From 095529f55cbbe34fa20a3ce00a3d50742810480e Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 09:18:16 -0800 Subject: [PATCH 11/42] flask-testing casing --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index bffd649f7f159..ff85770fea5ab 100644 --- a/setup.py +++ b/setup.py @@ -110,7 +110,7 @@ def get_git_sha(): extras_require={ "bigquery": ["pybigquery>=0.4.10", "pandas_gbq>=0.10.0"], "cors": ["flask-cors>=2.0.0"], - "flask-testing": ["Flask-Testing==0.7.1"], + "flask-testing": ["flask-testing==0.7.1"], "gsheets": ["gsheetsdb>=0.1.9"], "hive": ["pyhive[hive]>=0.6.1", "tableschema", "thrift>=0.11.0, <1.0.0"], "mysql": ["mysqlclient==1.4.2.post1"], From 428655ba3b686076bbd6b6f3541f116b34c57239 Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 09:26:04 -0800 Subject: [PATCH 12/42] resultsbackend property bug --- superset/utils/results_backend_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/utils/results_backend_manager.py b/superset/utils/results_backend_manager.py index 8317f753be77e..321f0d51c2c5a 100644 --- a/superset/utils/results_backend_manager.py +++ b/superset/utils/results_backend_manager.py @@ -28,7 +28,7 @@ def init_app(self, app): @property def results_backend(self): - return self.results_backend + return self._results_backend @property def should_use_msgpack(self): From 97e4cdac0bc5761c91ef34b8bf948930a05ba1f2 Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 11:22:53 -0800 Subject: [PATCH 13/42] Fixing up cli --- superset/cli.py | 31 +++++++++++++++++++++++++++---- tests/base_tests.py | 2 -- tests/load_examples_test.py | 23 +++++++++++++++-------- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/superset/cli.py b/superset/cli.py index f4794a24eb070..b0c11a414aa5e 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -61,6 +61,7 @@ def init(): @superset.command() +@with_appcontext @click.option("--verbose", "-v", is_flag=True, help="Show extra information") def version(verbose): """Prints the current version number""" @@ -69,7 +70,7 @@ def version(verbose): Fore.YELLOW + "Superset " + Fore.CYAN - + "{version}".format(version=config["VERSION_STRING"]) + + "{version}".format(version=app.config["VERSION_STRING"]) ) print(Fore.BLUE + "-=" * 15) if verbose: @@ -84,6 +85,8 @@ def load_examples_run(load_test_data, only_metadata=False, force=False): examples_db = utils.get_example_database() print(f"Loading examples metadata and related data into {examples_db}") + from superset import examples + examples.load_css_templates() print("Loading energy related dataset") @@ -136,6 +139,7 @@ def load_examples_run(load_test_data, only_metadata=False, force=False): examples.load_tabbed_dashboard(only_metadata) +@with_appcontext @superset.command() @click.option("--load-test-data", "-t", is_flag=True, help="Load additional test data") @click.option( @@ -149,6 +153,7 @@ def load_examples(load_test_data, only_metadata=False, force=False): load_examples_run(load_test_data, only_metadata, force) +@with_appcontext @superset.command() @click.option("--database_name", "-d", help="Database name to change") @click.option("--uri", "-u", help="Database URI to change") @@ -158,6 +163,7 @@ def set_database_uri(database_name, uri): @superset.command() +@with_appcontext @click.option( "--datasource", "-d", @@ -188,6 +194,7 @@ def refresh_druid(datasource, merge): @superset.command() +@with_appcontext @click.option( "--path", "-p", @@ -209,6 +216,7 @@ def refresh_druid(datasource, merge): ) def import_dashboards(path, recursive, username): """Import dashboards from JSON""" + from superset.utils import dashboard_import_export p = Path(path) files = [] if p.is_file(): @@ -230,6 +238,7 @@ def import_dashboards(path, recursive, username): @superset.command() +@with_appcontext @click.option( "--dashboard-file", "-f", default=None, help="Specify the the file to export to" ) @@ -238,6 +247,7 @@ def import_dashboards(path, recursive, username): ) def export_dashboards(print_stdout, dashboard_file): """Export dashboards to JSON""" + from superset.utils import dashboard_import_export data = dashboard_import_export.export_dashboards(db.session) if print_stdout or not dashboard_file: print(data) @@ -248,6 +258,7 @@ def export_dashboards(print_stdout, dashboard_file): @superset.command() +@with_appcontext @click.option( "--path", "-p", @@ -272,6 +283,8 @@ def export_dashboards(print_stdout, dashboard_file): ) def import_datasources(path, sync, recursive): """Import datasources from YAML""" + from superset.utils import dict_import_export + sync_array = sync.split(",") p = Path(path) files = [] @@ -296,6 +309,7 @@ def import_datasources(path, sync, recursive): @superset.command() +@with_appcontext @click.option( "--datasource-file", "-f", default=None, help="Specify the the file to export to" ) @@ -320,6 +334,7 @@ def export_datasources( print_stdout, datasource_file, back_references, include_defaults ): """Export datasources to YAML""" + from superset.utils import dict_import_export data = dict_import_export.export_to_dict( session=db.session, recursive=True, @@ -335,6 +350,7 @@ def export_datasources( @superset.command() +@with_appcontext @click.option( "--back-references", "-b", @@ -344,11 +360,13 @@ def export_datasources( ) def export_datasource_schema(back_references): """Export datasource YAML schema to stdout""" + from superset.utils import dict_import_export data = dict_import_export.export_schema_to_dict(back_references=back_references) yaml.safe_dump(data, stdout, default_flow_style=False) @superset.command() +@with_appcontext def update_datasources_cache(): """Refresh sqllab datasources cache""" from superset.models.core import Database @@ -368,6 +386,7 @@ def update_datasources_cache(): @superset.command() +@with_appcontext @click.option( "--workers", "-w", type=int, help="Number of celery server workers to fire up" ) @@ -379,14 +398,15 @@ def worker(workers): ) if workers: celery_app.conf.update(CELERYD_CONCURRENCY=workers) - elif config["SUPERSET_CELERY_WORKERS"]: - celery_app.conf.update(CELERYD_CONCURRENCY=config["SUPERSET_CELERY_WORKERS"]) + elif app.config["SUPERSET_CELERY_WORKERS"]: + celery_app.conf.update(CELERYD_CONCURRENCY=app.config["SUPERSET_CELERY_WORKERS"]) worker = celery_app.Worker(optimization="fair") worker.start() @superset.command() +@with_appcontext @click.option( "-p", "--port", default="5555", help="Port on which to start the Flower process" ) @@ -417,6 +437,7 @@ def flower(port, address): @superset.command() +@with_appcontext def load_test_users(): """ Loads admin, alpha, and gamma user for testing purposes @@ -469,12 +490,14 @@ def load_test_users_run(): ) sm.get_session.commit() - @superset.command() +@with_appcontext def sync_tags(): """Rebuilds special tags (owner, type, favorited by).""" # pylint: disable=no-member metadata = Model.metadata + + from superset.common.tags import add_favorites, add_owners, add_types add_types(db.engine, metadata) add_owners(db.engine, metadata) add_favorites(db.engine, metadata) diff --git a/tests/base_tests.py b/tests/base_tests.py index 217ae64fbf368..b2a6a3b70d5ae 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -17,7 +17,6 @@ """Unit tests for Superset""" import imp import json -import unittest from unittest.mock import Mock import pandas as pd @@ -25,7 +24,6 @@ from flask_testing import TestCase from superset import app, db, security_manager -from superset.app import create_app from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models diff --git a/tests/load_examples_test.py b/tests/load_examples_test.py index d98542f4f2c21..fd0fc26f3a322 100644 --- a/tests/load_examples_test.py +++ b/tests/load_examples_test.py @@ -14,27 +14,34 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from superset import examples -from superset.cli import load_test_users_run - from .base_tests import SupersetTestCase class SupersetDataFrameTestCase(SupersetTestCase): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.examples = None + + def setUp(self) -> None: + # Late importing here as we need an app context to be pushed... + from superset import examples + self.examples = examples + def test_load_css_templates(self): - examples.load_css_templates() + self.examples.load_css_templates() def test_load_energy(self): - examples.load_energy() + self.examples.load_energy() def test_load_world_bank_health_n_pop(self): - examples.load_world_bank_health_n_pop() + self.examples.load_world_bank_health_n_pop() def test_load_birth_names(self): - examples.load_birth_names() + self.examples.load_birth_names() def test_load_test_users_run(self): + from superset.cli import load_test_users_run load_test_users_run() def test_load_unicode_test_data(self): - examples.load_unicode_test_data() + self.examples.load_unicode_test_data() \ No newline at end of file From 9f686cded83b4c40543b8b9e1c3eebfafe608fcf Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 13:59:19 -0800 Subject: [PATCH 14/42] Quick fix for KV api --- superset/views/core.py | 4 +++- tests/access_tests.py | 31 +++++++++++++++++-------------- tests/base_tests.py | 2 +- tests/core_tests.py | 16 ++++++---------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index e15964105788a..8c50d9d793a01 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -672,7 +672,9 @@ def store(self): def get_value(self, key_id): kv = None try: - kv = db.session.query(models.KeyValue).filter_by(id=key_id).one() + kv = db.session.query(models.KeyValue).filter_by(id=key_id).scalar() + if not kv: + return Response(status=404, content_type="text/plain") except Exception as e: return json_error_response(e) return Response(kv.value, status=200, content_type="text/plain") diff --git a/tests/access_tests.py b/tests/access_tests.py index a27000ac29dd9..fe52985eb56a9 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -19,11 +19,12 @@ import unittest from unittest import mock -from superset import app, db, security_manager +from superset import db, security_manager from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.druid.models import DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models +from tests import app from .base_tests import SupersetTestCase @@ -94,22 +95,24 @@ def create_access_request(session, ds_type, ds_name, role_name, user_name): class RequestAccessTests(SupersetTestCase): @classmethod def setUpClass(cls): - security_manager.add_role("override_me") - security_manager.add_role(TEST_ROLE_1) - security_manager.add_role(TEST_ROLE_2) - security_manager.add_role(DB_ACCESS_ROLE) - security_manager.add_role(SCHEMA_ACCESS_ROLE) - db.session.commit() + with app.app_context(): + security_manager.add_role("override_me") + security_manager.add_role(TEST_ROLE_1) + security_manager.add_role(TEST_ROLE_2) + security_manager.add_role(DB_ACCESS_ROLE) + security_manager.add_role(SCHEMA_ACCESS_ROLE) + db.session.commit() @classmethod def tearDownClass(cls): - override_me = security_manager.find_role("override_me") - db.session.delete(override_me) - db.session.delete(security_manager.find_role(TEST_ROLE_1)) - db.session.delete(security_manager.find_role(TEST_ROLE_2)) - db.session.delete(security_manager.find_role(DB_ACCESS_ROLE)) - db.session.delete(security_manager.find_role(SCHEMA_ACCESS_ROLE)) - db.session.commit() + with app.app_context(): + override_me = security_manager.find_role("override_me") + db.session.delete(override_me) + db.session.delete(security_manager.find_role(TEST_ROLE_1)) + db.session.delete(security_manager.find_role(TEST_ROLE_2)) + db.session.delete(security_manager.find_role(DB_ACCESS_ROLE)) + db.session.delete(security_manager.find_role(SCHEMA_ACCESS_ROLE)) + db.session.commit() def setUp(self): self.login("admin") diff --git a/tests/base_tests.py b/tests/base_tests.py index b2a6a3b70d5ae..0d05f92195ed4 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -29,6 +29,7 @@ from superset.models import core as models from superset.models.core import Database from superset.utils.core import get_example_database +from tests import app class SupersetTestCase(TestCase): @@ -38,7 +39,6 @@ def __init__(self, *args, **kwargs): self.BASE_DIR = "" def create_app(self): - from tests import app return app def setUp(self) -> None: diff --git a/tests/core_tests.py b/tests/core_tests.py index 9b546d772b3eb..7813ff8e76e86 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -402,6 +402,10 @@ def test_databaseview_edit(self, username="admin"): database = utils.get_example_database() self.assertEqual(sqlalchemy_uri_decrypted, database.sqlalchemy_uri_decrypted) + # Need to clean up after ourselves + database.impersonate_user = False + db.session.commit() + def test_warm_up_cache(self): slc = self.get_slice("Girls", db.session) data = self.get_json_resp("/superset/warm_up_cache?slice_id={}".format(slc.id)) @@ -426,13 +430,10 @@ def test_shortner(self): assert re.search(r"\/r\/[0-9]+", resp.data.decode("utf-8")) def test_kv(self): - self.logout() self.login(username="admin") - try: - resp = self.client.post("/kv/store/", data=dict()) - except Exception: - self.assertRaises(TypeError) + resp = self.client.get("/kv/10001/") + self.assertEqual(404, resp.status_code) value = json.dumps({"data": "this is a test"}) resp = self.client.post("/kv/store/", data=dict(data=value)) @@ -445,11 +446,6 @@ def test_kv(self): self.assertEqual(resp.status_code, 200) self.assertEqual(json.loads(value), json.loads(resp.data.decode("utf-8"))) - try: - resp = self.client.get("/kv/10001/") - except Exception: - self.assertRaises(TypeError) - def test_gamma(self): self.login(username="gamma") assert "Charts" in self.get_resp("/chart/list/") From f1318d56fd3244e7baa378fea44158872a30b342 Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 14:45:58 -0800 Subject: [PATCH 15/42] Working on save slice --- tests/core_tests.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/core_tests.py b/tests/core_tests.py index 7813ff8e76e86..c279fa351fae1 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -192,12 +192,11 @@ def assert_admin_view_menus_in(role_name, assert_func): def test_save_slice(self): self.login(username="admin") - slice_name = "Energy Sankey" + slice_name = f"Energy Sankey" slice_id = self.get_slice(slice_name, db.session).id - db.session.commit() - copy_name = "Test Sankey Save" + copy_name = f"Test Sankey Save_{random.random()}" tbl_id = self.table_ids.get("energy_usage") - new_slice_name = "Test Sankey Overwirte" + new_slice_name = f"Test Sankey Overwrite_{random.random()}" url = ( "/superset/explore/table/{}/?slice_name={}&" @@ -217,8 +216,13 @@ def test_save_slice(self): {"form_data": json.dumps(form_data)}, ) slices = db.session.query(models.Slice).filter_by(slice_name=copy_name).all() - assert len(slices) == 1 - new_slice_id = slices[0].id + self.assertEqual(1, len(slices)) + slc = slices[0] + new_slice_id = slc.id + + self.assertEqual(slc.slice_name, copy_name) + form_data.pop("slice_id") # We don't save the slice id + self.assertEqual(slc.viz.form_data, form_data) form_data = { "viz_type": "sankey", @@ -234,8 +238,9 @@ def test_save_slice(self): {"form_data": json.dumps(form_data)}, ) slc = db.session.query(models.Slice).filter_by(id=new_slice_id).first() - assert slc.slice_name == new_slice_name - assert slc.viz.form_data == form_data + self.assertEqual(slc.slice_name, new_slice_name) + form_data.pop("slice_id") # We don't save the slice id + self.assertEqual(slc.viz.form_data, form_data) db.session.delete(slc) def test_filter_endpoint(self): From 947622b52b4f1c3caa55189966b6e1f56ec4298f Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 16:05:30 -0800 Subject: [PATCH 16/42] Fixed core_tests --- superset/views/core.py | 3 +++ tests/core_tests.py | 26 ++++++++++++++------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 8c50d9d793a01..c9799896da635 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -724,6 +724,8 @@ def shortner(self): class Superset(BaseSupersetView): """The base views for Superset!""" + logger = logging.getLogger(__name__) + @has_access_api @expose("/datasources/") def datasources(self): @@ -2037,6 +2039,7 @@ def warm_up_cache(self): ) obj.get_json() except Exception as e: + self.logger.exception("Failed to warm up cache") return json_error_response(utils.error_msg_from_exception(e)) return json_success( json.dumps( diff --git a/tests/core_tests.py b/tests/core_tests.py index c279fa351fae1..73e98c5e5c864 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -211,17 +211,16 @@ def test_save_slice(self): "slice_id": slice_id, } # Changing name and save as a new slice - self.get_resp( + resp = self.client.post( url.format(tbl_id, copy_name, "saveas"), - {"form_data": json.dumps(form_data)}, + data={"form_data": json.dumps(form_data)}, ) - slices = db.session.query(models.Slice).filter_by(slice_name=copy_name).all() - self.assertEqual(1, len(slices)) - slc = slices[0] - new_slice_id = slc.id + db.session.expunge_all() + new_slice_id = resp.json["form_data"]["slice_id"] + slc = db.session.query(models.Slice).filter_by(id=new_slice_id).one() self.assertEqual(slc.slice_name, copy_name) - form_data.pop("slice_id") # We don't save the slice id + form_data.pop("slice_id") # We don't save the slice id when saving as self.assertEqual(slc.viz.form_data, form_data) form_data = { @@ -233,15 +232,18 @@ def test_save_slice(self): "time_range": "now", } # Setting the name back to its original name by overwriting new slice - self.get_resp( + self.client.post( url.format(tbl_id, new_slice_name, "overwrite"), - {"form_data": json.dumps(form_data)}, + data={"form_data": json.dumps(form_data)}, ) - slc = db.session.query(models.Slice).filter_by(id=new_slice_id).first() + db.session.expunge_all() + slc = db.session.query(models.Slice).filter_by(id=new_slice_id).one() self.assertEqual(slc.slice_name, new_slice_name) - form_data.pop("slice_id") # We don't save the slice id self.assertEqual(slc.viz.form_data, form_data) + + # Cleanup db.session.delete(slc) + db.session.commit() def test_filter_endpoint(self): self.login(username="admin") @@ -414,7 +416,7 @@ def test_databaseview_edit(self, username="admin"): def test_warm_up_cache(self): slc = self.get_slice("Girls", db.session) data = self.get_json_resp("/superset/warm_up_cache?slice_id={}".format(slc.id)) - assert data == [{"slice_id": slc.id, "slice_name": slc.slice_name}] + self.assertEqual(data, [{"slice_id": slc.id, "slice_name": slc.slice_name}]) data = self.get_json_resp( "/superset/warm_up_cache?table_name=energy_usage&db_name=main" From 91a39f5aefea6e18e215123d4cc43cdd56a869df Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 8 Nov 2019 16:23:31 -0800 Subject: [PATCH 17/42] Fixed utils_tests --- superset/app.py | 1 + superset/utils/feature_flag_manager.py | 2 +- tests/utils_tests.py | 13 +++++++------ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/superset/app.py b/superset/app.py index 7eeff58dba9b5..b18f597168c60 100644 --- a/superset/app.py +++ b/superset/app.py @@ -105,6 +105,7 @@ def init_app_in_ctx(self) -> None: """ Runs init logic in the context of the app """ + self.configure_feature_flags() self.configure_fab() self.configure_data_sources() diff --git a/superset/utils/feature_flag_manager.py b/superset/utils/feature_flag_manager.py index 0bc849b65cbdd..2dc869ce410a4 100644 --- a/superset/utils/feature_flag_manager.py +++ b/superset/utils/feature_flag_manager.py @@ -22,7 +22,7 @@ class FeatureFlagManager: def __init__(self) -> None: super().__init__() self._get_feature_flags_func = None - self._feature_flags = {} + self._feature_flags = None def init_app(self, app): self._get_feature_flags_func = app.config.get("GET_FEATURE_FLAGS_FUNC") diff --git a/tests/utils_tests.py b/tests/utils_tests.py index 81adb93a0a01a..d13687bdd37d6 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -28,6 +28,7 @@ from superset import app, db, security_manager from superset.exceptions import SupersetException from superset.models.core import Database +from superset.utils.cache_manager import CacheManager from superset.utils.core import ( base_json_conv, convert_legacy_filters_into_adhoc, @@ -45,7 +46,6 @@ parse_human_timedelta, parse_js_uri_path_item, parse_past_timedelta, - setup_cache, split, TimeRangeEndpoint, validate_json, @@ -53,6 +53,7 @@ zlib_decompress, ) from superset.views.utils import get_time_range_endpoints +from tests.base_tests import SupersetTestCase def mock_parse_human_datetime(s): @@ -93,7 +94,7 @@ def mock_to_adhoc(filt, expressionType="SIMPLE", clause="where"): return result -class UtilsTestCase(unittest.TestCase): +class UtilsTestCase(SupersetTestCase): def test_json_int_dttm_ser(self): dttm = datetime(2020, 1, 1) ts = 1577836800000.0 @@ -809,12 +810,12 @@ def test_parse_js_uri_path_items_item_optional(self): def test_setup_cache_no_config(self): app = Flask(__name__) cache_config = None - self.assertIsNone(setup_cache(app, cache_config)) + self.assertIsNone(CacheManager._setup_cache(app, cache_config)) def test_setup_cache_null_config(self): app = Flask(__name__) cache_config = {"CACHE_TYPE": "null"} - self.assertIsNone(setup_cache(app, cache_config)) + self.assertIsNone(CacheManager._setup_cache(app, cache_config)) def test_setup_cache_standard_config(self): app = Flask(__name__) @@ -824,7 +825,7 @@ def test_setup_cache_standard_config(self): "CACHE_KEY_PREFIX": "superset_results", "CACHE_REDIS_URL": "redis://localhost:6379/0", } - assert isinstance(setup_cache(app, cache_config), Cache) is True + assert isinstance(CacheManager._setup_cache(app, cache_config), Cache) is True def test_setup_cache_custom_function(self): app = Flask(__name__) @@ -833,7 +834,7 @@ def test_setup_cache_custom_function(self): def init_cache(app): return CustomCache(app, {}) - assert isinstance(setup_cache(app, init_cache), CustomCache) is True + assert isinstance(CacheManager._setup_cache(app, init_cache), CustomCache) is True def test_get_stacktrace(self): with app.app_context(): From f675ad4e51d035d6bd3b7a98bacc0d1bde41e41c Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 10:44:51 -0800 Subject: [PATCH 18/42] Most tests working - still need to dig into remaining app_context issue in tests --- .flaskenv | 2 +- tests/__init__.py | 8 +- tests/base_tests.py | 41 ++++--- tests/celery_tests.py | 36 +++---- tests/dashboard_tests.py | 18 ++-- tests/db_engine_specs/presto_tests.py | 18 ++-- tests/dict_import_export_tests.py | 20 ++-- tests/druid_func_tests.py | 2 +- tests/email_tests.py | 3 +- tests/import_export_tests.py | 148 +++++++++++++------------- tests/schedules_test.py | 62 ++++++----- tests/security_tests.py | 1 + tests/sql_validator_tests.py | 4 +- tests/sqllab_tests.py | 9 +- tests/utils_tests.py | 2 + 15 files changed, 188 insertions(+), 186 deletions(-) diff --git a/.flaskenv b/.flaskenv index 33adde0c5eb85..ac72d0aa7b00b 100644 --- a/.flaskenv +++ b/.flaskenv @@ -14,5 +14,5 @@ # See the License for the specific language governing permissions and # limitations under the License. # -FLASK_APP=superset:app +FLASK_APP=superset:app.create_app() FLASK_ENV=development diff --git a/tests/__init__.py b/tests/__init__.py index 17826c80897b0..83f07d02c0590 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -15,14 +15,12 @@ # specific language governing permissions and limitations # under the License. +from os import environ +environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") + """ TODO: Clean this up! The current pattern of accessing app props on package init means that we need to ensure the creation of our Flask app BEFORE any tests load """ -from os import environ -environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") - from superset.app import create_app app = create_app() -# ctx = app.test_request_context() -# ctx.push() diff --git a/tests/base_tests.py b/tests/base_tests.py index 0d05f92195ed4..edff6dcef0a38 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -23,7 +23,7 @@ from flask_appbuilder.security.sqla import models as ab_models from flask_testing import TestCase -from superset import app, db, security_manager +from superset import db, security_manager from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models @@ -36,35 +36,32 @@ class SupersetTestCase(TestCase): def __init__(self, *args, **kwargs): super(SupersetTestCase, self).__init__(*args, **kwargs) self.maxDiff = None - self.BASE_DIR = "" def create_app(self): return app - def setUp(self) -> None: - self.BASE_DIR = app.config["BASE_DIR"] - @classmethod def create_druid_test_objects(cls): # create druid cluster and druid datasources - session = db.session - cluster = ( - session.query(DruidCluster).filter_by(cluster_name="druid_test").first() - ) - if not cluster: - cluster = DruidCluster(cluster_name="druid_test") - session.add(cluster) - session.commit() - - druid_datasource1 = DruidDatasource( - datasource_name="druid_ds_1", cluster_name="druid_test" - ) - session.add(druid_datasource1) - druid_datasource2 = DruidDatasource( - datasource_name="druid_ds_2", cluster_name="druid_test" + with app.app_context(): + session = db.session + cluster = ( + session.query(DruidCluster).filter_by(cluster_name="druid_test").first() ) - session.add(druid_datasource2) - session.commit() + if not cluster: + cluster = DruidCluster(cluster_name="druid_test") + session.add(cluster) + session.commit() + + druid_datasource1 = DruidDatasource( + datasource_name="druid_ds_1", cluster_name="druid_test" + ) + session.add(druid_datasource1) + druid_datasource2 = DruidDatasource( + datasource_name="druid_ds_2", cluster_name="druid_test" + ) + session.add(druid_datasource2) + session.commit() def get_table(self, table_id): return db.session.query(SqlaTable).filter_by(id=table_id).one() diff --git a/tests/celery_tests.py b/tests/celery_tests.py index a90fd957680b6..3ffb916f6e36a 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -22,30 +22,20 @@ import unittest import unittest.mock as mock -from superset import app, db, sql_lab +from superset import db, sql_lab from superset.dataframe import SupersetDataFrame from superset.db_engine_specs.base import BaseEngineSpec from superset.models.helpers import QueryStatus from superset.models.sql_lab import Query from superset.sql_parse import ParsedQuery from superset.utils.core import get_example_database +from tests import app from .base_tests import SupersetTestCase -BASE_DIR = app.config["BASE_DIR"] CELERY_SLEEP_TIME = 5 -class CeleryConfig(object): - BROKER_URL = app.config["CELERY_CONFIG"].BROKER_URL - CELERY_IMPORTS = ("superset.sql_lab",) - CELERY_ANNOTATIONS = {"sql_lab.add": {"rate_limit": "10/s"}} - CONCURRENCY = 1 - - -app.config["CELERY_CONFIG"] = CeleryConfig - - class UtilityFunctionTests(SupersetTestCase): # TODO(bkyryliuk): support more cases in CTA function. @@ -79,10 +69,6 @@ def test_create_table_as(self): class CeleryTestCase(SupersetTestCase): - def __init__(self, *args, **kwargs): - super(CeleryTestCase, self).__init__(*args, **kwargs) - self.client = app.test_client() - def get_query_by_name(self, sql): session = db.session query = session.query(Query).filter_by(sql=sql).first() @@ -97,11 +83,21 @@ def get_query_by_id(self, id): @classmethod def setUpClass(cls): - db.session.query(Query).delete() - db.session.commit() + with app.app_context(): + class CeleryConfig(object): + BROKER_URL = app.config["CELERY_CONFIG"].BROKER_URL + CELERY_IMPORTS = ("superset.sql_lab",) + CELERY_ANNOTATIONS = {"sql_lab.add": {"rate_limit": "10/s"}} + CONCURRENCY = 1 + + app.config["CELERY_CONFIG"] = CeleryConfig + + db.session.query(Query).delete() + db.session.commit() - worker_command = BASE_DIR + "/bin/superset worker -w 2" - subprocess.Popen(worker_command, shell=True, stdout=subprocess.PIPE) + base_dir = app.config["BASE_DIR"] + worker_command = base_dir + "/bin/superset worker -w 2" + subprocess.Popen(worker_command, shell=True, stdout=subprocess.PIPE) @classmethod def tearDownClass(cls): diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index 6753b9ae7ca51..59c88c41111d5 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -17,6 +17,7 @@ """Unit tests for Superset""" import json import unittest +from random import random from flask import escape from sqlalchemy import func @@ -402,13 +403,13 @@ def test_users_can_view_published_dashboard(self): # Create a published and hidden dashboard and add them to the database published_dash = models.Dashboard() published_dash.dashboard_title = "Published Dashboard" - published_dash.slug = "published_dash" + published_dash.slug = f"published_dash_{random()}" published_dash.slices = [slice] published_dash.published = True hidden_dash = models.Dashboard() hidden_dash.dashboard_title = "Hidden Dashboard" - hidden_dash.slug = "hidden_dash" + hidden_dash.slug = f"hidden_dash_{random()}" hidden_dash.slices = [slice] hidden_dash.published = False @@ -426,13 +427,13 @@ def test_users_can_view_own_dashboard(self): # Create one dashboard I own and another that I don't dash = models.Dashboard() dash.dashboard_title = "My Dashboard" - dash.slug = "my_dash" + dash.slug = f"my_dash_{random()}" dash.owners = [user] dash.slices = [] hidden_dash = models.Dashboard() hidden_dash.dashboard_title = "Not My Dashboard" - hidden_dash.slug = "not_my_dash" + hidden_dash.slug = f"not_my_dash_{random()}" hidden_dash.slices = [] hidden_dash.owners = [] @@ -448,14 +449,15 @@ def test_users_can_view_own_dashboard(self): def test_users_can_view_favorited_dashboards(self): user = security_manager.find_user("gamma") + fav_dash_slug = f"my_favorite_dash_{random()}" favorite_dash = models.Dashboard() favorite_dash.dashboard_title = "My Favorite Dashboard" - favorite_dash.slug = "my_favorite_dash" + favorite_dash.slug = fav_dash_slug regular_dash = models.Dashboard() regular_dash.dashboard_title = "A Plain Ol Dashboard" - regular_dash.slug = "regular_dash" + regular_dash.slug = f"regular_dash_{random()}" db.session.merge(favorite_dash) db.session.merge(regular_dash) @@ -463,7 +465,7 @@ def test_users_can_view_favorited_dashboards(self): dash = ( db.session.query(models.Dashboard) - .filter_by(slug="my_favorite_dash") + .filter_by(slug=fav_dash_slug) .first() ) @@ -483,7 +485,7 @@ def test_users_can_view_favorited_dashboards(self): def test_user_can_not_view_unpublished_dash(self): admin_user = security_manager.find_user("admin") gamma_user = security_manager.find_user("gamma") - slug = "admin_owned_unpublished_dash" + slug = f"admin_owned_unpublished_dash_{random()}" # Create a dashboard owned by admin and unpublished dash = models.Dashboard() diff --git a/tests/db_engine_specs/presto_tests.py b/tests/db_engine_specs/presto_tests.py index 48ad18a244503..ba36572b83913 100644 --- a/tests/db_engine_specs/presto_tests.py +++ b/tests/db_engine_specs/presto_tests.py @@ -60,7 +60,7 @@ def test_presto_get_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_simple_row_column(self): presto_column = ("column_name", "row(nested_obj double)", "") @@ -68,7 +68,7 @@ def test_presto_get_simple_row_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_simple_row_column_with_name_containing_whitespace(self): presto_column = ("column name", "row(nested_obj double)", "") @@ -76,7 +76,7 @@ def test_presto_get_simple_row_column_with_name_containing_whitespace(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_simple_row_column_with_tricky_nested_field_name(self): presto_column = ("column_name", 'row("Field Name(Tricky, Name)" double)', "") @@ -87,7 +87,7 @@ def test_presto_get_simple_row_column_with_tricky_nested_field_name(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_simple_array_column(self): presto_column = ("column_name", "array(double)", "") @@ -95,7 +95,7 @@ def test_presto_get_simple_array_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_row_within_array_within_row_column(self): presto_column = ( @@ -112,7 +112,7 @@ def test_presto_get_row_within_array_within_row_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_get_array_within_row_within_array_column(self): presto_column = ( @@ -147,7 +147,7 @@ def test_presto_get_fields(self): self.assertEqual(actual_result.name, expected_result["label"]) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_expand_data_with_simple_structural_columns(self): cols = [ @@ -182,7 +182,7 @@ def test_presto_expand_data_with_simple_structural_columns(self): self.assertEqual(actual_expanded_cols, expected_expanded_cols) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_expand_data_with_complex_row_columns(self): cols = [ @@ -229,7 +229,7 @@ def test_presto_expand_data_with_complex_row_columns(self): self.assertEqual(actual_expanded_cols, expected_expanded_cols) @mock.patch.dict( - "superset._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True ) def test_presto_expand_data_with_complex_array_columns(self): cols = [ diff --git a/tests/dict_import_export_tests.py b/tests/dict_import_export_tests.py index 425eac283fb79..1e08144f43b2f 100644 --- a/tests/dict_import_export_tests.py +++ b/tests/dict_import_export_tests.py @@ -24,6 +24,7 @@ from superset.connectors.druid.models import DruidColumn, DruidDatasource, DruidMetric from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn from superset.utils.core import get_example_database +from tests import app from .base_tests import SupersetTestCase @@ -40,15 +41,16 @@ def __init__(self, *args, **kwargs): @classmethod def delete_imports(cls): - # Imported data clean up - session = db.session - for table in session.query(SqlaTable): - if DBREF in table.params_dict: - session.delete(table) - for datasource in session.query(DruidDatasource): - if DBREF in datasource.params_dict: - session.delete(datasource) - session.commit() + with app.app_context(): + # Imported data clean up + session = db.session + for table in session.query(SqlaTable): + if DBREF in table.params_dict: + session.delete(table) + for datasource in session.query(DruidDatasource): + if DBREF in datasource.params_dict: + session.delete(datasource) + session.commit() @classmethod def setUpClass(cls): diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py index 3afbfaccada49..508d45f3f7827 100644 --- a/tests/druid_func_tests.py +++ b/tests/druid_func_tests.py @@ -47,7 +47,7 @@ def emplace(metrics_dict, metric_name, is_postagg=False): # Unit tests that can be run without initializing base tests -class DruidFuncTestCase(unittest.TestCase): +class DruidFuncTestCase(SupersetTestCase): @unittest.skipUnless( SupersetTestCase.is_module_installed("pydruid"), "pydruid not installed" ) diff --git a/tests/email_tests.py b/tests/email_tests.py index ba4c2638e84f6..e8fd3578886e9 100644 --- a/tests/email_tests.py +++ b/tests/email_tests.py @@ -26,13 +26,14 @@ from superset import app from superset.utils import core as utils +from tests.base_tests import SupersetTestCase from .utils import read_fixture send_email_test = mock.Mock() -class EmailSmtpTest(unittest.TestCase): +class EmailSmtpTest(SupersetTestCase): def setUp(self): app.config["smtp_ssl"] = False diff --git a/tests/import_export_tests.py b/tests/import_export_tests.py index c4f032fccc534..7b899b48133d5 100644 --- a/tests/import_export_tests.py +++ b/tests/import_export_tests.py @@ -26,6 +26,7 @@ from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn from superset.models import core as models from superset.utils import core as utils +from tests import app from .base_tests import SupersetTestCase @@ -35,21 +36,22 @@ class ImportExportTests(SupersetTestCase): @classmethod def delete_imports(cls): - # Imported data clean up - session = db.session - for slc in session.query(models.Slice): - if "remote_id" in slc.params_dict: - session.delete(slc) - for dash in session.query(models.Dashboard): - if "remote_id" in dash.params_dict: - session.delete(dash) - for table in session.query(SqlaTable): - if "remote_id" in table.params_dict: - session.delete(table) - for datasource in session.query(DruidDatasource): - if "remote_id" in datasource.params_dict: - session.delete(datasource) - session.commit() + with app.app_context(): + # Imported data clean up + session = db.session + for slc in session.query(models.Slice): + if "remote_id" in slc.params_dict: + session.delete(slc) + for dash in session.query(models.Dashboard): + if "remote_id" in dash.params_dict: + session.delete(dash) + for table in session.query(SqlaTable): + if "remote_id" in table.params_dict: + session.delete(table) + for datasource in session.query(DruidDatasource): + if "remote_id" in datasource.params_dict: + session.delete(datasource) + session.commit() @classmethod def setUpClass(cls): @@ -460,68 +462,64 @@ def test_import_override_dashboard_2_slices(self): ) def test_import_new_dashboard_slice_reset_ownership(self): - app = Flask("test_import_dashboard_slice_set_user") - with app.app_context(): - admin_user = security_manager.find_user(username="admin") - self.assertTrue(admin_user) - gamma_user = security_manager.find_user(username="gamma") - self.assertTrue(gamma_user) - g.user = gamma_user - - dash_with_1_slice = self._create_dashboard_for_import(id_=10200) - # set another user as an owner of importing dashboard - dash_with_1_slice.created_by = admin_user - dash_with_1_slice.changed_by = admin_user - dash_with_1_slice.owners = [admin_user] - - imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) - imported_dash = self.get_dash(imported_dash_id) - self.assertEqual(imported_dash.created_by, gamma_user) - self.assertEqual(imported_dash.changed_by, gamma_user) - self.assertEqual(imported_dash.owners, [gamma_user]) - - imported_slc = imported_dash.slices[0] - self.assertEqual(imported_slc.created_by, gamma_user) - self.assertEqual(imported_slc.changed_by, gamma_user) - self.assertEqual(imported_slc.owners, [gamma_user]) + admin_user = security_manager.find_user(username="admin") + self.assertTrue(admin_user) + gamma_user = security_manager.find_user(username="gamma") + self.assertTrue(gamma_user) + g.user = gamma_user + + dash_with_1_slice = self._create_dashboard_for_import(id_=10200) + # set another user as an owner of importing dashboard + dash_with_1_slice.created_by = admin_user + dash_with_1_slice.changed_by = admin_user + dash_with_1_slice.owners = [admin_user] + + imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) + imported_dash = self.get_dash(imported_dash_id) + self.assertEqual(imported_dash.created_by, gamma_user) + self.assertEqual(imported_dash.changed_by, gamma_user) + self.assertEqual(imported_dash.owners, [gamma_user]) + + imported_slc = imported_dash.slices[0] + self.assertEqual(imported_slc.created_by, gamma_user) + self.assertEqual(imported_slc.changed_by, gamma_user) + self.assertEqual(imported_slc.owners, [gamma_user]) def test_import_override_dashboard_slice_reset_ownership(self): - app = Flask("test_import_dashboard_slice_set_user") - with app.app_context(): - admin_user = security_manager.find_user(username="admin") - self.assertTrue(admin_user) - gamma_user = security_manager.find_user(username="gamma") - self.assertTrue(gamma_user) - g.user = gamma_user - - dash_with_1_slice = self._create_dashboard_for_import(id_=10300) - - imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) - imported_dash = self.get_dash(imported_dash_id) - self.assertEqual(imported_dash.created_by, gamma_user) - self.assertEqual(imported_dash.changed_by, gamma_user) - self.assertEqual(imported_dash.owners, [gamma_user]) - - imported_slc = imported_dash.slices[0] - self.assertEqual(imported_slc.created_by, gamma_user) - self.assertEqual(imported_slc.changed_by, gamma_user) - self.assertEqual(imported_slc.owners, [gamma_user]) - - # re-import with another user shouldn't change the permissions - g.user = admin_user - - dash_with_1_slice = self._create_dashboard_for_import(id_=10300) - - imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) - imported_dash = self.get_dash(imported_dash_id) - self.assertEqual(imported_dash.created_by, gamma_user) - self.assertEqual(imported_dash.changed_by, gamma_user) - self.assertEqual(imported_dash.owners, [gamma_user]) - - imported_slc = imported_dash.slices[0] - self.assertEqual(imported_slc.created_by, gamma_user) - self.assertEqual(imported_slc.changed_by, gamma_user) - self.assertEqual(imported_slc.owners, [gamma_user]) + admin_user = security_manager.find_user(username="admin") + self.assertTrue(admin_user) + gamma_user = security_manager.find_user(username="gamma") + self.assertTrue(gamma_user) + g.user = gamma_user + + dash_with_1_slice = self._create_dashboard_for_import(id_=10300) + + imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) + imported_dash = self.get_dash(imported_dash_id) + self.assertEqual(imported_dash.created_by, gamma_user) + self.assertEqual(imported_dash.changed_by, gamma_user) + self.assertEqual(imported_dash.owners, [gamma_user]) + + imported_slc = imported_dash.slices[0] + self.assertEqual(imported_slc.created_by, gamma_user) + self.assertEqual(imported_slc.changed_by, gamma_user) + self.assertEqual(imported_slc.owners, [gamma_user]) + + # re-import with another user shouldn't change the permissions + g.user = admin_user + + dash_with_1_slice = self._create_dashboard_for_import(id_=10300) + + imported_dash_id = models.Dashboard.import_obj(dash_with_1_slice) + imported_dash = self.get_dash(imported_dash_id) + self.assertEqual(imported_dash.created_by, gamma_user) + self.assertEqual(imported_dash.changed_by, gamma_user) + self.assertEqual(imported_dash.owners, [gamma_user]) + + imported_slc = imported_dash.slices[0] + self.assertEqual(imported_slc.created_by, gamma_user) + self.assertEqual(imported_slc.changed_by, gamma_user) + self.assertEqual(imported_slc.owners, [gamma_user]) def _create_dashboard_for_import(self, id_=10100): slc = self.create_slice("health_slc" + str(id_), id=id_ + 1) diff --git a/tests/schedules_test.py b/tests/schedules_test.py index e6ad92d65d626..805485a0bd05e 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -21,7 +21,7 @@ from flask_babel import gettext as __ from selenium.common.exceptions import WebDriverException -from superset import app, db +from superset import db from superset.models.core import Dashboard, Slice from superset.models.schedules import ( DashboardEmailSchedule, @@ -35,11 +35,13 @@ deliver_slice, next_schedules, ) +from tests import app +from tests.base_tests import SupersetTestCase from .utils import read_fixture -class SchedulesTestCase(unittest.TestCase): +class SchedulesTestCase(SupersetTestCase): RECIPIENTS = "recipient1@superset.com, recipient2@superset.com" BCC = "bcc@superset.com" @@ -47,41 +49,43 @@ class SchedulesTestCase(unittest.TestCase): @classmethod def setUpClass(cls): - cls.common_data = dict( - active=True, - crontab="* * * * *", - recipients=cls.RECIPIENTS, - deliver_as_group=True, - delivery_type=EmailDeliveryType.inline, - ) + with app.app_context(): + cls.common_data = dict( + active=True, + crontab="* * * * *", + recipients=cls.RECIPIENTS, + deliver_as_group=True, + delivery_type=EmailDeliveryType.inline, + ) - # Pick up a random slice and dashboard - slce = db.session.query(Slice).all()[0] - dashboard = db.session.query(Dashboard).all()[0] + # Pick up a random slice and dashboard + slce = db.session.query(Slice).all()[0] + dashboard = db.session.query(Dashboard).all()[0] - dashboard_schedule = DashboardEmailSchedule(**cls.common_data) - dashboard_schedule.dashboard_id = dashboard.id - dashboard_schedule.user_id = 1 - db.session.add(dashboard_schedule) + dashboard_schedule = DashboardEmailSchedule(**cls.common_data) + dashboard_schedule.dashboard_id = dashboard.id + dashboard_schedule.user_id = 1 + db.session.add(dashboard_schedule) - slice_schedule = SliceEmailSchedule(**cls.common_data) - slice_schedule.slice_id = slce.id - slice_schedule.user_id = 1 - slice_schedule.email_format = SliceEmailReportFormat.data + slice_schedule = SliceEmailSchedule(**cls.common_data) + slice_schedule.slice_id = slce.id + slice_schedule.user_id = 1 + slice_schedule.email_format = SliceEmailReportFormat.data - db.session.add(slice_schedule) - db.session.commit() + db.session.add(slice_schedule) + db.session.commit() - cls.slice_schedule = slice_schedule.id - cls.dashboard_schedule = dashboard_schedule.id + cls.slice_schedule = slice_schedule.id + cls.dashboard_schedule = dashboard_schedule.id @classmethod def tearDownClass(cls): - db.session.query(SliceEmailSchedule).filter_by(id=cls.slice_schedule).delete() - db.session.query(DashboardEmailSchedule).filter_by( - id=cls.dashboard_schedule - ).delete() - db.session.commit() + with app.app_context(): + db.session.query(SliceEmailSchedule).filter_by(id=cls.slice_schedule).delete() + db.session.query(DashboardEmailSchedule).filter_by( + id=cls.dashboard_schedule + ).delete() + db.session.commit() def test_crontab_scheduler(self): crontab = "* * * * *" diff --git a/tests/security_tests.py b/tests/security_tests.py index 4478389902fc9..0391d0d51e7ba 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -310,6 +310,7 @@ def test_views_are_secured(self): ["Superset", "welcome"], ["SecurityApi", "login"], ["SecurityApi", "refresh"], + ["SupersetIndexView", "index"], ] unsecured_views = [] for view_class in appbuilder.baseviews: diff --git a/tests/sql_validator_tests.py b/tests/sql_validator_tests.py index 069bae8a6bc20..be7864e1f905c 100644 --- a/tests/sql_validator_tests.py +++ b/tests/sql_validator_tests.py @@ -60,7 +60,7 @@ def test_validate_sql_endpoint_noconfig(self): self.assertIn("no SQL validator is configured", resp["error"]) @patch("superset.views.core.get_validator_by_name") - @patch.dict("superset._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) def test_validate_sql_endpoint_mocked(self, get_validator_by_name): """Assert that, with a mocked validator, annotations make it back out from the validate_sql_json endpoint as a list of json dictionaries""" @@ -87,7 +87,7 @@ def test_validate_sql_endpoint_mocked(self, get_validator_by_name): self.assertIn("expected,", resp[0]["message"]) @patch("superset.views.core.get_validator_by_name") - @patch.dict("superset._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) + @patch.dict("superset.extensions.feature_flag_manager._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) def test_validate_sql_endpoint_failure(self, get_validator_by_name): """Assert that validate_sql_json errors out when the selected validator raises an unexpected exception""" diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py index 806f6018a9907..35a63a74c7ec0 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -17,10 +17,11 @@ """Unit tests for Sql Lab""" import json from datetime import datetime, timedelta +from random import random import prison -from superset import db, security_manager +from superset import db, security_manager, ConnectorRegistry from superset.dataframe import SupersetDataFrame from superset.db_engine_specs import BaseEngineSpec from superset.models.sql_lab import Query @@ -292,19 +293,19 @@ def test_sqllab_viz(self): examples_dbid = get_example_database().id payload = { "chartType": "dist_bar", - "datasourceName": "test_viz_flow_table", + "datasourceName": f"test_viz_flow_table_{random()}", "schema": "superset", "columns": [ { "is_date": False, "type": "STRING", - "name": "viz_type", + "name": f"viz_type_{random()}", "is_dim": True, }, { "is_date": False, "type": "OBJECT", - "name": "ccount", + "name": f"ccount_{random()}", "is_dim": True, "agg": "sum", }, diff --git a/tests/utils_tests.py b/tests/utils_tests.py index d13687bdd37d6..d070553c47588 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -880,6 +880,8 @@ def test_get_or_create_db(self): get_or_create_db("test_db", "sqlite:///changed.db") database = db.session.query(Database).filter_by(database_name="test_db").one() self.assertEqual(database.sqlalchemy_uri, "sqlite:///changed.db") + db.session.delete(database) + db.session.commit() def test_get_or_create_db_invalid_uri(self): with self.assertRaises(ArgumentError): From 78990d1e38d7d3eb395baa6f2bf474aa5e18a996 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 11:23:03 -0800 Subject: [PATCH 19/42] All tests passing locally - need to update code comments --- tests/dashboard_tests.py | 11 +++++++---- tox.ini | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index 59c88c41111d5..b833e0c05d373 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -400,16 +400,19 @@ def test_users_can_view_published_dashboard(self): self.grant_public_access_to_table(table) + hidden_dash_slug = f"hidden_dash_{random()}" + published_dash_slug = f"published_dash_{random()}" + # Create a published and hidden dashboard and add them to the database published_dash = models.Dashboard() published_dash.dashboard_title = "Published Dashboard" - published_dash.slug = f"published_dash_{random()}" + published_dash.slug = published_dash_slug published_dash.slices = [slice] published_dash.published = True hidden_dash = models.Dashboard() hidden_dash.dashboard_title = "Hidden Dashboard" - hidden_dash.slug = f"hidden_dash_{random()}" + hidden_dash.slug = hidden_dash_slug hidden_dash.slices = [slice] hidden_dash.published = False @@ -418,8 +421,8 @@ def test_users_can_view_published_dashboard(self): db.session.commit() resp = self.get_resp("/dashboard/list/") - self.assertNotIn("/superset/dashboard/hidden_dash/", resp) - self.assertIn("/superset/dashboard/published_dash/", resp) + self.assertNotIn(f"/superset/dashboard/{hidden_dash_slug}/", resp) + self.assertIn(f"/superset/dashboard/{published_dash_slug}/", resp) def test_users_can_view_own_dashboard(self): user = security_manager.find_user("gamma") diff --git a/tox.ini b/tox.ini index 8a8096df6c0fa..81408e158efca 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ commands = {toxinidir}/superset/bin/superset db upgrade {toxinidir}/superset/bin/superset init nosetests tests/load_examples_test.py - nosetests -e load_examples_test {posargs} + nosetests -e load_examples_test tests {posargs} deps = -rrequirements.txt -rrequirements-dev.txt From 7e37cef25098c0953a95fdb702a66741a017dfc5 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 11:43:07 -0800 Subject: [PATCH 20/42] Fixing dashboard tests again --- tests/dashboard_tests.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index b833e0c05d373..bfa0ee5f4664a 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -426,17 +426,19 @@ def test_users_can_view_published_dashboard(self): def test_users_can_view_own_dashboard(self): user = security_manager.find_user("gamma") + my_dash_slug = f"my_dash_{random()}" + not_my_dash_slug = f"not_my_dash_{random()}" # Create one dashboard I own and another that I don't dash = models.Dashboard() dash.dashboard_title = "My Dashboard" - dash.slug = f"my_dash_{random()}" + dash.slug = my_dash_slug dash.owners = [user] dash.slices = [] hidden_dash = models.Dashboard() hidden_dash.dashboard_title = "Not My Dashboard" - hidden_dash.slug = f"not_my_dash_{random()}" + hidden_dash.slug = not_my_dash_slug hidden_dash.slices = [] hidden_dash.owners = [] @@ -447,12 +449,13 @@ def test_users_can_view_own_dashboard(self): self.login(user.username) resp = self.get_resp("/dashboard/list/") - self.assertIn("/superset/dashboard/my_dash/", resp) - self.assertNotIn("/superset/dashboard/not_my_dash/", resp) + self.assertIn(f"/superset/dashboard/{my_dash_slug}/", resp) + self.assertNotIn(f"/superset/dashboard/{not_my_dash_slug}/", resp) def test_users_can_view_favorited_dashboards(self): user = security_manager.find_user("gamma") fav_dash_slug = f"my_favorite_dash_{random()}" + regular_dash_slug = f"regular_dash_{random()}" favorite_dash = models.Dashboard() favorite_dash.dashboard_title = "My Favorite Dashboard" @@ -460,7 +463,7 @@ def test_users_can_view_favorited_dashboards(self): regular_dash = models.Dashboard() regular_dash.dashboard_title = "A Plain Ol Dashboard" - regular_dash.slug = f"regular_dash_{random()}" + regular_dash.slug = regular_dash_slug db.session.merge(favorite_dash) db.session.merge(regular_dash) @@ -483,7 +486,7 @@ def test_users_can_view_favorited_dashboards(self): self.login(user.username) resp = self.get_resp("/dashboard/list/") - self.assertIn("/superset/dashboard/my_favorite_dash/", resp) + self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", resp) def test_user_can_not_view_unpublished_dash(self): admin_user = security_manager.find_user("admin") From 3b95e96ae4e5bf171d56adb9d0775ad39d217c99 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 12:01:15 -0800 Subject: [PATCH 21/42] Blacking --- superset/__init__.py | 6 +++-- superset/app.py | 11 +++++--- superset/cli.py | 13 ++++++++-- superset/utils/cache.py | 4 ++- tests/__init__.py | 11 +++++--- tests/celery_tests.py | 1 + tests/dashboard_tests.py | 6 +---- tests/db_engine_specs/presto_tests.py | 36 ++++++++++++++++++++------- tests/feature_flag_tests.py | 10 ++++++-- tests/load_examples_test.py | 4 ++- tests/schedules_test.py | 4 ++- tests/sql_validator_tests.py | 12 +++++++-- tests/utils_tests.py | 4 ++- 13 files changed, 89 insertions(+), 33 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 2aa623b72937b..6eb6559b9c2f3 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -29,7 +29,7 @@ manifest_processor as ext_manifest_processor, results_backend_manager as ext_results_backend_manager, security_manager as ext_security_manager, - talisman as ext_talisman + talisman as ext_talisman, ) from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value @@ -44,7 +44,9 @@ get_css_manifest_files = ext_manifest_processor.get_css_manifest_files is_feature_enabled = ext_feature_flag_manager.is_feature_enabled results_backend = LocalProxy(lambda: ext_results_backend_manager.results_backend) -results_backend_use_msgpack = LocalProxy(lambda: ext_results_backend_manager.should_use_msgpack) +results_backend_use_msgpack = LocalProxy( + lambda: ext_results_backend_manager.should_use_msgpack +) security_manager = ext_security_manager tables_cache = LocalProxy(lambda: ext_cache_manager.tables_cache) talisman = ext_talisman diff --git a/superset/app.py b/superset/app.py index b18f597168c60..e39f9831a0631 100644 --- a/superset/app.py +++ b/superset/app.py @@ -32,7 +32,8 @@ migrate, results_backend_manager, talisman, - celery_app) + celery_app, +) from flask_wtf import CSRFProtect from superset.connectors.connector_registry import ConnectorRegistry @@ -194,7 +195,9 @@ def configure_middlewares(self): if self.config["ENABLE_PROXY_FIX"]: from werkzeug.middleware.proxy_fix import ProxyFix - self.flask_app.wsgi_app = ProxyFix(self.flask_app.wsgi_app, **self.config["PROXY_FIX_CONFIG"]) + self.flask_app.wsgi_app = ProxyFix( + self.flask_app.wsgi_app, **self.config["PROXY_FIX_CONFIG"] + ) if self.config["ENABLE_CHUNK_ENCODING"]: @@ -229,8 +232,8 @@ def __call__(self, environ, start_response): def configure_logging(self): self.flask_app.config.get("LOGGING_CONFIGURATOR").configure_logging( - self.config, - self.flask_app.debug) + self.config, self.flask_app.debug + ) def setup_db(self): db.init_app(self.flask_app) diff --git a/superset/cli.py b/superset/cli.py index b0c11a414aa5e..bc9cadf2318bf 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -34,7 +34,7 @@ # from superset.extensions import celery_app # from superset.utils import core as utils, dashboard_import_export, dict_import_export -#config = app.config +# config = app.config from superset import app, appbuilder, security_manager from superset.app import create_app @@ -46,6 +46,7 @@ @with_appcontext def superset(): """This is a management script for the Superset application.""" + @app.shell_context_processor def make_shell_context(): return dict(app=app, db=db) @@ -217,6 +218,7 @@ def refresh_druid(datasource, merge): def import_dashboards(path, recursive, username): """Import dashboards from JSON""" from superset.utils import dashboard_import_export + p = Path(path) files = [] if p.is_file(): @@ -248,6 +250,7 @@ def import_dashboards(path, recursive, username): def export_dashboards(print_stdout, dashboard_file): """Export dashboards to JSON""" from superset.utils import dashboard_import_export + data = dashboard_import_export.export_dashboards(db.session) if print_stdout or not dashboard_file: print(data) @@ -335,6 +338,7 @@ def export_datasources( ): """Export datasources to YAML""" from superset.utils import dict_import_export + data = dict_import_export.export_to_dict( session=db.session, recursive=True, @@ -361,6 +365,7 @@ def export_datasources( def export_datasource_schema(back_references): """Export datasource YAML schema to stdout""" from superset.utils import dict_import_export + data = dict_import_export.export_schema_to_dict(back_references=back_references) yaml.safe_dump(data, stdout, default_flow_style=False) @@ -399,7 +404,9 @@ def worker(workers): if workers: celery_app.conf.update(CELERYD_CONCURRENCY=workers) elif app.config["SUPERSET_CELERY_WORKERS"]: - celery_app.conf.update(CELERYD_CONCURRENCY=app.config["SUPERSET_CELERY_WORKERS"]) + celery_app.conf.update( + CELERYD_CONCURRENCY=app.config["SUPERSET_CELERY_WORKERS"] + ) worker = celery_app.Worker(optimization="fair") worker.start() @@ -490,6 +497,7 @@ def load_test_users_run(): ) sm.get_session.commit() + @superset.command() @with_appcontext def sync_tags(): @@ -498,6 +506,7 @@ def sync_tags(): metadata = Model.metadata from superset.common.tags import add_favorites, add_owners, add_types + add_types(db.engine, metadata) add_owners(db.engine, metadata) add_favorites(db.engine, metadata) diff --git a/superset/utils/cache.py b/superset/utils/cache.py index 2fe8a819048e9..23905e2d906a7 100644 --- a/superset/utils/cache.py +++ b/superset/utils/cache.py @@ -61,7 +61,9 @@ def wrapped_f(self, *args, **kwargs): if not kwargs.get("force") and o is not None: return o o = f(self, *args, **kwargs) - cache_manager.tables_cache.set(cache_key, o, timeout=kwargs.get("cache_timeout")) + cache_manager.tables_cache.set( + cache_key, o, timeout=kwargs.get("cache_timeout") + ) return o else: diff --git a/tests/__init__.py b/tests/__init__.py index 83f07d02c0590..5ceb75fa71d44 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -16,11 +16,16 @@ # under the License. from os import environ + +from superset.app import create_app + +""" +Setting this envvar forces unit tests to use the "test" config for all tests +""" environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") """ -TODO: Clean this up! The current pattern of accessing app props on package init means - that we need to ensure the creation of our Flask app BEFORE any tests load +Here is where we create the app which ends up being shared across all tests. A future +optimization will be to create a separate app instance for each test class. """ -from superset.app import create_app app = create_app() diff --git a/tests/celery_tests.py b/tests/celery_tests.py index 3ffb916f6e36a..623e048259a35 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -84,6 +84,7 @@ def get_query_by_id(self, id): @classmethod def setUpClass(cls): with app.app_context(): + class CeleryConfig(object): BROKER_URL = app.config["CELERY_CONFIG"].BROKER_URL CELERY_IMPORTS = ("superset.sql_lab",) diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index bfa0ee5f4664a..4dca9bf156635 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -469,11 +469,7 @@ def test_users_can_view_favorited_dashboards(self): db.session.merge(regular_dash) db.session.commit() - dash = ( - db.session.query(models.Dashboard) - .filter_by(slug=fav_dash_slug) - .first() - ) + dash = db.session.query(models.Dashboard).filter_by(slug=fav_dash_slug).first() favorites = models.FavStar() favorites.obj_id = dash.id diff --git a/tests/db_engine_specs/presto_tests.py b/tests/db_engine_specs/presto_tests.py index ba36572b83913..bfb032294bee8 100644 --- a/tests/db_engine_specs/presto_tests.py +++ b/tests/db_engine_specs/presto_tests.py @@ -60,7 +60,9 @@ def test_presto_get_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_simple_row_column(self): presto_column = ("column_name", "row(nested_obj double)", "") @@ -68,7 +70,9 @@ def test_presto_get_simple_row_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_simple_row_column_with_name_containing_whitespace(self): presto_column = ("column name", "row(nested_obj double)", "") @@ -76,7 +80,9 @@ def test_presto_get_simple_row_column_with_name_containing_whitespace(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_simple_row_column_with_tricky_nested_field_name(self): presto_column = ("column_name", 'row("Field Name(Tricky, Name)" double)', "") @@ -87,7 +93,9 @@ def test_presto_get_simple_row_column_with_tricky_nested_field_name(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_simple_array_column(self): presto_column = ("column_name", "array(double)", "") @@ -95,7 +103,9 @@ def test_presto_get_simple_array_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_row_within_array_within_row_column(self): presto_column = ( @@ -112,7 +122,9 @@ def test_presto_get_row_within_array_within_row_column(self): self.verify_presto_column(presto_column, expected_results) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_get_array_within_row_within_array_column(self): presto_column = ( @@ -147,7 +159,9 @@ def test_presto_get_fields(self): self.assertEqual(actual_result.name, expected_result["label"]) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_expand_data_with_simple_structural_columns(self): cols = [ @@ -182,7 +196,9 @@ def test_presto_expand_data_with_simple_structural_columns(self): self.assertEqual(actual_expanded_cols, expected_expanded_cols) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_expand_data_with_complex_row_columns(self): cols = [ @@ -229,7 +245,9 @@ def test_presto_expand_data_with_complex_row_columns(self): self.assertEqual(actual_expanded_cols, expected_expanded_cols) @mock.patch.dict( - "superset.extensions.feature_flag_manager._feature_flags", {"PRESTO_EXPAND_DATA": True}, clear=True + "superset.extensions.feature_flag_manager._feature_flags", + {"PRESTO_EXPAND_DATA": True}, + clear=True, ) def test_presto_expand_data_with_complex_array_columns(self): cols = [ diff --git a/tests/feature_flag_tests.py b/tests/feature_flag_tests.py index f698016637d9f..36524fd369311 100644 --- a/tests/feature_flag_tests.py +++ b/tests/feature_flag_tests.py @@ -4,11 +4,17 @@ class FeatureFlagTests(SupersetTestCase): - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {"FOO": True}, clear=True) + @patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + {"FOO": True}, + clear=True, + ) def test_existing_feature_flags(self): self.assertTrue(is_feature_enabled("FOO")) - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", {}, clear=True) + @patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", {}, clear=True + ) def test_nonexistent_feature_flags(self): self.assertFalse(is_feature_enabled("FOO")) diff --git a/tests/load_examples_test.py b/tests/load_examples_test.py index fd0fc26f3a322..5b61730163d32 100644 --- a/tests/load_examples_test.py +++ b/tests/load_examples_test.py @@ -25,6 +25,7 @@ def __init__(self, *args, **kwargs): def setUp(self) -> None: # Late importing here as we need an app context to be pushed... from superset import examples + self.examples = examples def test_load_css_templates(self): @@ -41,7 +42,8 @@ def test_load_birth_names(self): def test_load_test_users_run(self): from superset.cli import load_test_users_run + load_test_users_run() def test_load_unicode_test_data(self): - self.examples.load_unicode_test_data() \ No newline at end of file + self.examples.load_unicode_test_data() diff --git a/tests/schedules_test.py b/tests/schedules_test.py index 805485a0bd05e..4e0bf2118323b 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -81,7 +81,9 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): with app.app_context(): - db.session.query(SliceEmailSchedule).filter_by(id=cls.slice_schedule).delete() + db.session.query(SliceEmailSchedule).filter_by( + id=cls.slice_schedule + ).delete() db.session.query(DashboardEmailSchedule).filter_by( id=cls.dashboard_schedule ).delete() diff --git a/tests/sql_validator_tests.py b/tests/sql_validator_tests.py index be7864e1f905c..553e799039fa8 100644 --- a/tests/sql_validator_tests.py +++ b/tests/sql_validator_tests.py @@ -60,7 +60,11 @@ def test_validate_sql_endpoint_noconfig(self): self.assertIn("no SQL validator is configured", resp["error"]) @patch("superset.views.core.get_validator_by_name") - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) + @patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + PRESTO_TEST_FEATURE_FLAGS, + clear=True, + ) def test_validate_sql_endpoint_mocked(self, get_validator_by_name): """Assert that, with a mocked validator, annotations make it back out from the validate_sql_json endpoint as a list of json dictionaries""" @@ -87,7 +91,11 @@ def test_validate_sql_endpoint_mocked(self, get_validator_by_name): self.assertIn("expected,", resp[0]["message"]) @patch("superset.views.core.get_validator_by_name") - @patch.dict("superset.extensions.feature_flag_manager._feature_flags", PRESTO_TEST_FEATURE_FLAGS, clear=True) + @patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + PRESTO_TEST_FEATURE_FLAGS, + clear=True, + ) def test_validate_sql_endpoint_failure(self, get_validator_by_name): """Assert that validate_sql_json errors out when the selected validator raises an unexpected exception""" diff --git a/tests/utils_tests.py b/tests/utils_tests.py index d070553c47588..b44282cade4da 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -834,7 +834,9 @@ def test_setup_cache_custom_function(self): def init_cache(app): return CustomCache(app, {}) - assert isinstance(CacheManager._setup_cache(app, init_cache), CustomCache) is True + assert ( + isinstance(CacheManager._setup_cache(app, init_cache), CustomCache) is True + ) def test_get_stacktrace(self): with app.app_context(): From 7f1dadcbc5abf80d374f50258fec34d4cceb67af Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 12:01:49 -0800 Subject: [PATCH 22/42] Sorting imports --- superset/app.py | 17 ++++++++--------- superset/bin/superset | 1 - superset/cli.py | 9 +++++---- superset/extensions.py | 3 +-- superset/sql_lab.py | 2 +- superset/tasks/cache.py | 2 +- superset/tasks/schedules.py | 2 +- superset/utils/cache.py | 3 ++- tests/feature_flag_tests.py | 3 ++- tests/sqllab_tests.py | 2 +- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/superset/app.py b/superset/app.py index e39f9831a0631..aa69a54512b01 100644 --- a/superset/app.py +++ b/superset/app.py @@ -18,30 +18,29 @@ import logging import os -from flask_appbuilder import IndexView, expose -from flask_compress import Compress +import wtforms_json from flask import Flask, redirect +from flask_appbuilder import expose, IndexView +from flask_compress import Compress +from flask_wtf import CSRFProtect + +from superset.connectors.connector_registry import ConnectorRegistry from superset.extensions import ( _event_logger, APP_DIR, appbuilder, cache_manager, + celery_app, db, feature_flag_manager, manifest_processor, migrate, results_backend_manager, talisman, - celery_app, ) -from flask_wtf import CSRFProtect - -from superset.connectors.connector_registry import ConnectorRegistry from superset.security import SupersetSecurityManager from superset.utils.core import pessimistic_connection_handling -from superset.utils.log import get_event_logger_from_cfg_value, DBEventLogger -import wtforms_json - +from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value logger = logging.getLogger(__name__) diff --git a/superset/bin/superset b/superset/bin/superset index 0f5239726a2cc..4b37e8339b2c7 100755 --- a/superset/bin/superset +++ b/superset/bin/superset @@ -17,6 +17,5 @@ # under the License. from superset.cli import superset - if __name__ == '__main__': superset() diff --git a/superset/cli.py b/superset/cli.py index bc9cadf2318bf..7e813700ab1ae 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -29,6 +29,11 @@ from flask_appbuilder import Model from pathlib2 import Path +from superset import app, appbuilder, security_manager +from superset.app import create_app +from superset.extensions import celery_app, db +from superset.utils import core as utils + # from superset import app, appbuilder, db, examples, security_manager, create_app # from superset.common.tags import add_favorites, add_owners, add_types # from superset.extensions import celery_app @@ -36,10 +41,6 @@ # config = app.config -from superset import app, appbuilder, security_manager -from superset.app import create_app -from superset.extensions import celery_app, db -from superset.utils import core as utils @click.group(cls=FlaskGroup, create_app=create_app) diff --git a/superset/extensions.py b/superset/extensions.py index b27599c5f85a4..a8e8c4d2db9e4 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -24,9 +24,8 @@ from flask_talisman import Talisman from werkzeug.local import LocalProxy -from superset.utils.feature_flag_manager import FeatureFlagManager - from superset.utils.cache_manager import CacheManager +from superset.utils.feature_flag_manager import FeatureFlagManager from superset.utils.manifest_processor import UIManifestProcessor from superset.utils.results_backend_manager import ResultsBackendManager diff --git a/superset/sql_lab.py b/superset/sql_lab.py index 4434883d7ab99..1bddd7c8b83d5 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -42,9 +42,9 @@ ) from superset.dataframe import SupersetDataFrame from superset.db_engine_specs import BaseEngineSpec +from superset.extensions import celery_app from superset.models.sql_lab import Query from superset.sql_parse import ParsedQuery -from superset.extensions import celery_app from superset.utils.core import json_iso_dttm_ser, QueryStatus, sources, zlib_compress from superset.utils.dates import now_as_float from superset.utils.decorators import stats_timing diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index 94bb7307c2133..09cd1f344427c 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -25,9 +25,9 @@ from sqlalchemy import and_, func from superset import app, db +from superset.extensions import celery_app from superset.models.core import Dashboard, Log, Slice from superset.models.tags import Tag, TaggedObject -from superset.extensions import celery_app from superset.utils.core import parse_human_datetime logger = get_task_logger(__name__) diff --git a/superset/tasks/schedules.py b/superset/tasks/schedules.py index 98ab5166aff06..f4b7911d8924b 100644 --- a/superset/tasks/schedules.py +++ b/superset/tasks/schedules.py @@ -39,13 +39,13 @@ # Superset framework imports from superset import app, db, security_manager +from superset.extensions import celery_app from superset.models.schedules import ( EmailDeliveryType, get_scheduler_model, ScheduleType, SliceEmailReportFormat, ) -from superset.extensions import celery_app from superset.utils.core import get_email_address_list, send_email_smtp # Globals diff --git a/superset/utils/cache.py b/superset/utils/cache.py index 23905e2d906a7..88bc8703e887c 100644 --- a/superset/utils/cache.py +++ b/superset/utils/cache.py @@ -17,8 +17,9 @@ # pylint: disable=C,R,W from typing import Optional -from flask import request, Flask +from flask import Flask, request from flask_caching import Cache + from superset.extensions import cache_manager diff --git a/tests/feature_flag_tests.py b/tests/feature_flag_tests.py index 36524fd369311..30585f8f37191 100644 --- a/tests/feature_flag_tests.py +++ b/tests/feature_flag_tests.py @@ -1,6 +1,7 @@ +from unittest.mock import patch + from superset import is_feature_enabled from tests.base_tests import SupersetTestCase -from unittest.mock import patch class FeatureFlagTests(SupersetTestCase): diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py index 35a63a74c7ec0..bec018e2c9fc6 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -21,7 +21,7 @@ import prison -from superset import db, security_manager, ConnectorRegistry +from superset import ConnectorRegistry, db, security_manager from superset.dataframe import SupersetDataFrame from superset.db_engine_specs import BaseEngineSpec from superset.models.sql_lab import Query From 01607c4854852303eaf71c199fdf84b457e297fd Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 12:42:27 -0800 Subject: [PATCH 23/42] linting --- superset/__init__.py | 32 ++++++++++++++++------------ superset/app.py | 15 +++++++------ superset/cli.py | 1 - superset/extensions.py | 3 +-- superset/forms.py | 4 ---- superset/utils/manifest_processor.py | 6 +++--- 6 files changed, 30 insertions(+), 31 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index 6eb6559b9c2f3..e8743c136d462 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -15,38 +15,42 @@ # specific language governing permissions and limitations # under the License. """Package's main module!""" -from flask import current_app as flask_current_app +from flask import current_app, Flask from werkzeug.local import LocalProxy from superset.app import create_app from superset.connectors.connector_registry import ConnectorRegistry from superset.extensions import ( appbuilder as ab, - cache_manager as ext_cache_manager, + cache_manager, db as ext_db, event_logger as ext_event_logger, - feature_flag_manager as ext_feature_flag_manager, - manifest_processor as ext_manifest_processor, - results_backend_manager as ext_results_backend_manager, + feature_flag_manager, + manifest_processor, + results_backend_manager, security_manager as ext_security_manager, talisman as ext_talisman, ) from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value -app = flask_current_app +# All of the fields located here should be considered legacy. The correct way +# to declare "global" dependencies is to define it in extensions.py, +# then initialize it in app.create_app(). These fields will be removed +# in subsequent PRs as things are migrated towards the factory pattern +app: Flask = current_app appbuilder = ab -cache = LocalProxy(lambda: ext_cache_manager.cache) -conf = LocalProxy(lambda: flask_current_app.config) +cache = LocalProxy(lambda: cache_manager.cache) +conf = LocalProxy(lambda: current_app.config) db = ext_db event_logger = ext_event_logger -get_feature_flags = ext_feature_flag_manager.get_feature_flags -get_css_manifest_files = ext_manifest_processor.get_css_manifest_files -is_feature_enabled = ext_feature_flag_manager.is_feature_enabled -results_backend = LocalProxy(lambda: ext_results_backend_manager.results_backend) +get_feature_flags = feature_flag_manager.get_feature_flags +get_css_manifest_files = manifest_processor.get_css_manifest_files +is_feature_enabled = feature_flag_manager.is_feature_enabled +results_backend = LocalProxy(lambda: results_backend_manager.results_backend) results_backend_use_msgpack = LocalProxy( - lambda: ext_results_backend_manager.should_use_msgpack + lambda: results_backend_manager.should_use_msgpack ) security_manager = ext_security_manager -tables_cache = LocalProxy(lambda: ext_cache_manager.tables_cache) +tables_cache = LocalProxy(lambda: cache_manager.tables_cache) talisman = ext_talisman diff --git a/superset/app.py b/superset/app.py index aa69a54512b01..b4f595619ee95 100644 --- a/superset/app.py +++ b/superset/app.py @@ -76,7 +76,7 @@ def __init__(self, app: Flask) -> None: self.flask_app = app self.config = app.config - self.manifest = {} + self.manifest: dict = {} def pre_init(self) -> None: """ @@ -97,9 +97,10 @@ def configure_celery(self) -> None: celery_app.config_from_object(self.config["CELERY_CONFIG"]) celery_app.set_default() - def init_views(self) -> None: + @staticmethod + def init_views() -> None: # TODO - This should iterate over all views and register them with FAB... - from superset import views # noqa + from superset import views # noqa pylint: disable=unused-variable def init_app_in_ctx(self) -> None: """ @@ -200,14 +201,14 @@ def configure_middlewares(self): if self.config["ENABLE_CHUNK_ENCODING"]: - class ChunkedEncodingFix(object): + class ChunkedEncodingFix(object): # pylint: disable=too-few-public-methods def __init__(self, app): self.app = app def __call__(self, environ, start_response): # Setting wsgi.input_terminated tells werkzeug.wsgi to ignore # content-length and read the stream till the end. - if environ.get("HTTP_TRANSFER_ENCODING", "").lower() == u"chunked": + if environ.get("HTTP_TRANSFER_ENCODING", "").lower() == "chunked": environ["wsgi.input_terminated"] = True return self.app(environ, start_response) @@ -252,9 +253,9 @@ def configure_wtf(self): def register_blueprints(self): for bp in self.config["BLUEPRINTS"]: try: - logger.info("Registering blueprint: '{}'".format(bp.name)) + logger.info(f"Registering blueprint: '{bp.name}'") self.flask_app.register_blueprint(bp) - except Exception: + except Exception: # pylint: disable=broad-except logger.exception("blueprint registration failed") def setup_bundle_manifest(self): diff --git a/superset/cli.py b/superset/cli.py index 7e813700ab1ae..138c410e8edeb 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -42,7 +42,6 @@ # config = app.config - @click.group(cls=FlaskGroup, create_app=create_app) @with_appcontext def superset(): diff --git a/superset/extensions.py b/superset/extensions.py index a8e8c4d2db9e4..cc1c7b2f11824 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -18,7 +18,6 @@ import os import celery -from flask import Flask from flask_appbuilder import AppBuilder, SQLA from flask_migrate import Migrate from flask_talisman import Talisman @@ -35,7 +34,7 @@ cache_manager = CacheManager() celery_app = celery.Celery() db = SQLA() -_event_logger = {} +_event_logger: dict = {} event_logger = LocalProxy(lambda: _event_logger.get("event_logger")) feature_flag_manager = FeatureFlagManager() manifest_processor = UIManifestProcessor(APP_DIR) diff --git a/superset/forms.py b/superset/forms.py index c11bf7b258571..9bf781de8becb 100644 --- a/superset/forms.py +++ b/superset/forms.py @@ -19,10 +19,6 @@ from flask_appbuilder.fieldwidgets import BS3TextFieldWidget from wtforms import Field -from superset import app - -config = app.config - class CommaSeparatedListField(Field): widget = BS3TextFieldWidget() diff --git a/superset/utils/manifest_processor.py b/superset/utils/manifest_processor.py index 73362709ba71a..128cccf1ffb52 100644 --- a/superset/utils/manifest_processor.py +++ b/superset/utils/manifest_processor.py @@ -22,7 +22,7 @@ class UIManifestProcessor: def __init__(self, app_dir: str) -> None: super().__init__() self.app = None - self.manifest = {} + self.manifest: dict = {} self.manifest_file = f"{app_dir}/static/assets/dist/manifest.json" def init_app(self, app): @@ -31,7 +31,7 @@ def init_app(self, app): self.parse_manifest_json() @app.context_processor - def get_manifest(): + def get_manifest(): # pylint: disable=unused-variable return dict( loaded_chunks=set(), get_unloaded_chunks=self.get_unloaded_chunks, @@ -46,7 +46,7 @@ def parse_manifest_json(self): # we only need entries in templates full_manifest = json.load(f) self.manifest = full_manifest.get("entrypoints", {}) - except Exception: + except Exception: # pylint: disable=broad-except pass def get_js_manifest_files(self, filename): From 3a66b0712e9d4f91323d07cf8b5c01fbac10e409 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 13 Nov 2019 15:50:26 -0800 Subject: [PATCH 24/42] removing envvar mangling --- tests/__init__.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 5ceb75fa71d44..f1644e3f21445 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -15,17 +15,9 @@ # specific language governing permissions and limitations # under the License. -from os import environ - -from superset.app import create_app - -""" -Setting this envvar forces unit tests to use the "test" config for all tests -""" -environ.setdefault("SUPERSET_CONFIG", "tests.superset_test_config") - """ Here is where we create the app which ends up being shared across all tests. A future -optimization will be to create a separate app instance for each test class. +optimization will be to create a separate app instance for each test class. """ +from superset.app import create_app app = create_app() From 74cd1bffed71cd8d75692d91ca69f6ee505a2411 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 10:52:28 -0800 Subject: [PATCH 25/42] blacking --- tests/__init__.py | 1 + tests/core_tests.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index f1644e3f21445..f3e708b550fd3 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -20,4 +20,5 @@ optimization will be to create a separate app instance for each test class. """ from superset.app import create_app + app = create_app() diff --git a/tests/core_tests.py b/tests/core_tests.py index 92010265f83e4..9f891728c329b 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -950,7 +950,11 @@ def test_results_msgpack_deserialization(self): self.assertDictEqual(deserialized_payload, payload) expand_data.assert_called_once() - @mock.patch.dict("superset._feature_flags", {"FOO": lambda x: 1}, clear=True) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + {"FOO": lambda x: 1}, + clear=True, + ) def test_feature_flag_serialization(self): """ Functions in feature flags don't break bootstrap data serialization. From 8d53bd78ae6176fe91c151ba3b50f678f7fef634 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 11:28:18 -0800 Subject: [PATCH 26/42] Fixing unit tests --- tests/base_tests.py | 14 +++++++++++++- tests/core_tests.py | 3 +++ tests/sqllab_tests.py | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/base_tests.py b/tests/base_tests.py index edff6dcef0a38..450744d878fe0 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -32,6 +32,9 @@ from tests import app +FAKE_DB_NAME = "fake_db_100" + + class SupersetTestCase(TestCase): def __init__(self, *args, **kwargs): super(SupersetTestCase, self).__init__(*args, **kwargs) @@ -212,7 +215,7 @@ def run_sql( def create_fake_db(self): self.login(username="admin") - database_name = "fake_db_100" + database_name = FAKE_DB_NAME db_id = 100 extra = """{ "schemas_allowed_for_csv_upload": @@ -227,6 +230,15 @@ def create_fake_db(self): extra=extra, ) + def delete_fake_db(self): + database = ( + db.session.query(Database) + .filter(Database.database_name == FAKE_DB_NAME) + .scalar() + ) + if database: + db.session.delete(database) + def validate_sql( self, sql, diff --git a/tests/core_tests.py b/tests/core_tests.py index 9f891728c329b..ee7b33981687f 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -412,6 +412,8 @@ def test_databaseview_edit(self, username="admin"): # Need to clean up after ourselves database.impersonate_user = False + database.allow_dml = False + database.allow_run_async = False db.session.commit() def test_warm_up_cache(self): @@ -808,6 +810,7 @@ def test_schemas_access_for_csv_upload_endpoint( ) ) assert data == ["this_schema_is_allowed_too"] + self.delete_fake_db() def test_select_star(self): self.login(username="admin") diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py index bec018e2c9fc6..4c3662c3a5b97 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -415,3 +415,4 @@ def test_api_database(self): {"examples", "fake_db_100"}, {r.get("database_name") for r in self.get_json_resp(url)["result"]}, ) + self.delete_fake_db() From 037cb2c1f59160c9e09f55bd6fad7c8eab75c75c Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 13:16:58 -0800 Subject: [PATCH 27/42] isorting --- tests/base_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/base_tests.py b/tests/base_tests.py index 450744d878fe0..92e1e96bf6a46 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -31,7 +31,6 @@ from superset.utils.core import get_example_database from tests import app - FAKE_DB_NAME = "fake_db_100" From 3c0d7dadedca2a508ead5df47673c67cab16e799 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 13:33:02 -0800 Subject: [PATCH 28/42] licensing --- superset/utils/cache_manager.py | 16 ++++++++++++++++ superset/utils/feature_flag_manager.py | 1 - tests/feature_flag_tests.py | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py index 81536f9df17ac..cfbeb349978f5 100644 --- a/superset/utils/cache_manager.py +++ b/superset/utils/cache_manager.py @@ -1,3 +1,19 @@ +# 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 Optional from flask import Flask diff --git a/superset/utils/feature_flag_manager.py b/superset/utils/feature_flag_manager.py index 2dc869ce410a4..7802f65c3f6cc 100644 --- a/superset/utils/feature_flag_manager.py +++ b/superset/utils/feature_flag_manager.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - from copy import deepcopy diff --git a/tests/feature_flag_tests.py b/tests/feature_flag_tests.py index 30585f8f37191..8712c63657d44 100644 --- a/tests/feature_flag_tests.py +++ b/tests/feature_flag_tests.py @@ -1,3 +1,19 @@ +# 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 unittest.mock import patch from superset import is_feature_enabled From c3c480dc9d2fc144e4512f5d2a1fd61f10dde2f9 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 16:03:30 -0800 Subject: [PATCH 29/42] fixing mysql tests --- tests/celery_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/celery_tests.py b/tests/celery_tests.py index 623e048259a35..182a78556e09d 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -187,6 +187,7 @@ def test_run_async_query(self): result = self.run_sql( db_id, sql_where, "4", async_=True, tmp_table="tmp_async_1", cta=True ) + db.session.close() assert result["query"]["state"] in ( QueryStatus.PENDING, QueryStatus.RUNNING, @@ -221,6 +222,7 @@ def test_run_async_query_with_lower_limit(self): result = self.run_sql( db_id, sql_where, "5", async_=True, tmp_table=tmp_table, cta=True ) + db.session.close() assert result["query"]["state"] in ( QueryStatus.PENDING, QueryStatus.RUNNING, From 90a25f2759d82ff55e6a3fbec9fb8dd1297798d3 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 16:50:53 -0800 Subject: [PATCH 30/42] fixing cypress? --- .flaskenv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.flaskenv b/.flaskenv index ac72d0aa7b00b..f12f12266e83f 100644 --- a/.flaskenv +++ b/.flaskenv @@ -14,5 +14,5 @@ # See the License for the specific language governing permissions and # limitations under the License. # -FLASK_APP=superset:app.create_app() -FLASK_ENV=development +FLASK_APP="superset:app.create_app()" +FLASK_ENV="development" From d8e74a2919eed018668f00b4cf3545f7586977f8 Mon Sep 17 00:00:00 2001 From: Craig Date: Thu, 14 Nov 2019 20:30:19 -0800 Subject: [PATCH 31/42] fixing .flaskenv --- .flaskenv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.flaskenv b/.flaskenv index f12f12266e83f..49bf390fb3db5 100644 --- a/.flaskenv +++ b/.flaskenv @@ -14,5 +14,5 @@ # See the License for the specific language governing permissions and # limitations under the License. # -FLASK_APP="superset:app.create_app()" +FLASK_APP="superset.app:create_app()" FLASK_ENV="development" From 55d5c27a6abf88bbedf2e323b02aa97d37dd61ec Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 15 Nov 2019 09:23:13 -0800 Subject: [PATCH 32/42] fixing test app_ctx --- tests/__init__.py | 8 -------- tests/access_tests.py | 2 +- tests/base_tests.py | 2 +- tests/celery_tests.py | 2 +- tests/core_tests.py | 3 ++- tests/dict_import_export_tests.py | 2 +- tests/import_export_tests.py | 2 +- tests/schedules_test.py | 2 +- tests/test_app.py | 24 ++++++++++++++++++++++++ 9 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 tests/test_app.py diff --git a/tests/__init__.py b/tests/__init__.py index f3e708b550fd3..13a83393a9124 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -14,11 +14,3 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - -""" -Here is where we create the app which ends up being shared across all tests. A future -optimization will be to create a separate app instance for each test class. -""" -from superset.app import create_app - -app = create_app() diff --git a/tests/access_tests.py b/tests/access_tests.py index fe52985eb56a9..4b14f15a6e957 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -19,12 +19,12 @@ import unittest from unittest import mock +from tests.test_app import app # isort:skip from superset import db, security_manager from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.druid.models import DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models -from tests import app from .base_tests import SupersetTestCase diff --git a/tests/base_tests.py b/tests/base_tests.py index 92e1e96bf6a46..c6d882bdf6822 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -23,13 +23,13 @@ from flask_appbuilder.security.sqla import models as ab_models from flask_testing import TestCase +from tests.test_app import app # isort:skip from superset import db, security_manager from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models from superset.models.core import Database from superset.utils.core import get_example_database -from tests import app FAKE_DB_NAME = "fake_db_100" diff --git a/tests/celery_tests.py b/tests/celery_tests.py index 182a78556e09d..e2649a8db256e 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -22,6 +22,7 @@ import unittest import unittest.mock as mock +from tests.test_app import app # isort:skip from superset import db, sql_lab from superset.dataframe import SupersetDataFrame from superset.db_engine_specs.base import BaseEngineSpec @@ -29,7 +30,6 @@ from superset.models.sql_lab import Query from superset.sql_parse import ParsedQuery from superset.utils.core import get_example_database -from tests import app from .base_tests import SupersetTestCase diff --git a/tests/core_tests.py b/tests/core_tests.py index ee7b33981687f..1202777b31e00 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -33,7 +33,8 @@ import psycopg2 import sqlalchemy as sqla -from superset import app, dataframe, db, jinja_context, security_manager, sql_lab +from tests.test_app import app +from superset import dataframe, db, jinja_context, security_manager, sql_lab from superset.connectors.sqla.models import SqlaTable from superset.db_engine_specs.base import BaseEngineSpec from superset.db_engine_specs.mssql import MssqlEngineSpec diff --git a/tests/dict_import_export_tests.py b/tests/dict_import_export_tests.py index 09660cda09673..b314c211dc69b 100644 --- a/tests/dict_import_export_tests.py +++ b/tests/dict_import_export_tests.py @@ -20,12 +20,12 @@ import yaml +from tests.test_app import app from superset import db from superset.connectors.druid.models import DruidColumn, DruidDatasource, DruidMetric from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn from superset.utils.core import get_example_database from superset.utils.dict_import_export import export_to_dict -from tests import app from .base_tests import SupersetTestCase diff --git a/tests/import_export_tests.py b/tests/import_export_tests.py index 7b899b48133d5..50999b97882f5 100644 --- a/tests/import_export_tests.py +++ b/tests/import_export_tests.py @@ -21,12 +21,12 @@ from flask import Flask, g from sqlalchemy.orm.session import make_transient +from tests.test_app import app from superset import db, security_manager from superset.connectors.druid.models import DruidColumn, DruidDatasource, DruidMetric from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn from superset.models import core as models from superset.utils import core as utils -from tests import app from .base_tests import SupersetTestCase diff --git a/tests/schedules_test.py b/tests/schedules_test.py index 4e0bf2118323b..fc8586752fc67 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -21,6 +21,7 @@ from flask_babel import gettext as __ from selenium.common.exceptions import WebDriverException +from tests.test_app import app from superset import db from superset.models.core import Dashboard, Slice from superset.models.schedules import ( @@ -35,7 +36,6 @@ deliver_slice, next_schedules, ) -from tests import app from tests.base_tests import SupersetTestCase from .utils import read_fixture diff --git a/tests/test_app.py b/tests/test_app.py new file mode 100644 index 0000000000000..f3e708b550fd3 --- /dev/null +++ b/tests/test_app.py @@ -0,0 +1,24 @@ +# 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. + +""" +Here is where we create the app which ends up being shared across all tests. A future +optimization will be to create a separate app instance for each test class. +""" +from superset.app import create_app + +app = create_app() From 92e79223330ae3c1b69e570302a5aec9dfb6bebc Mon Sep 17 00:00:00 2001 From: Craig Date: Fri, 15 Nov 2019 09:52:50 -0800 Subject: [PATCH 33/42] fixing cypress --- tests/access_tests.py | 1 + tests/base_tests.py | 1 + tests/celery_tests.py | 1 + tests/core_tests.py | 1 + tests/dict_import_export_tests.py | 1 + tests/import_export_tests.py | 3 ++- tests/schedules_test.py | 2 +- 7 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/access_tests.py b/tests/access_tests.py index 4b14f15a6e957..7b0be43c042f4 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset""" import json import unittest diff --git a/tests/base_tests.py b/tests/base_tests.py index c6d882bdf6822..7399774cc8ef8 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset""" import imp import json diff --git a/tests/celery_tests.py b/tests/celery_tests.py index e2649a8db256e..954c84d5fade4 100644 --- a/tests/celery_tests.py +++ b/tests/celery_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset Celery worker""" import datetime import json diff --git a/tests/core_tests.py b/tests/core_tests.py index 1202777b31e00..81cd69ffa8c8f 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset""" import cgi import csv diff --git a/tests/dict_import_export_tests.py b/tests/dict_import_export_tests.py index b314c211dc69b..d30443e3c8076 100644 --- a/tests/dict_import_export_tests.py +++ b/tests/dict_import_export_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset""" import json import unittest diff --git a/tests/import_export_tests.py b/tests/import_export_tests.py index 50999b97882f5..2641e35adbdc9 100644 --- a/tests/import_export_tests.py +++ b/tests/import_export_tests.py @@ -14,11 +14,12 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +# isort:skip_file """Unit tests for Superset""" import json import unittest -from flask import Flask, g +from flask import g from sqlalchemy.orm.session import make_transient from tests.test_app import app diff --git a/tests/schedules_test.py b/tests/schedules_test.py index fc8586752fc67..da55f9efa287a 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import unittest +# isort:skip_file from datetime import datetime, timedelta from unittest.mock import Mock, patch, PropertyMock From b943d684d47d408f17dac3f2bf0a7b2d0429c17e Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 18 Nov 2019 12:20:11 -0800 Subject: [PATCH 34/42] moving manifest processor around --- superset/extensions.py | 55 +++++++++++++++++++++- superset/utils/manifest_processor.py | 69 ---------------------------- 2 files changed, 53 insertions(+), 71 deletions(-) delete mode 100644 superset/utils/manifest_processor.py diff --git a/superset/extensions.py b/superset/extensions.py index cc1c7b2f11824..4e8d0ba026fdc 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - +import json import os import celery @@ -25,9 +25,60 @@ from superset.utils.cache_manager import CacheManager from superset.utils.feature_flag_manager import FeatureFlagManager -from superset.utils.manifest_processor import UIManifestProcessor from superset.utils.results_backend_manager import ResultsBackendManager + +class UIManifestProcessor: + def __init__(self, app_dir: str) -> None: + super().__init__() + self.app = None + self.manifest: dict = {} + self.manifest_file = f"{app_dir}/static/assets/dist/manifest.json" + + def init_app(self, app): + self.app = app + # Preload the cache + self.parse_manifest_json() + + @app.context_processor + def get_manifest(): # pylint: disable=unused-variable + return dict( + loaded_chunks=set(), + get_unloaded_chunks=self.get_unloaded_chunks, + js_manifest=self.get_js_manifest_files, + css_manifest=self.get_css_manifest_files, + ) + + def parse_manifest_json(self): + try: + with open(self.manifest_file, "r") as f: + # the manifest includes non-entry files + # we only need entries in templates + full_manifest = json.load(f) + self.manifest = full_manifest.get("entrypoints", {}) + except Exception: # pylint: disable=broad-except + pass + + def get_js_manifest_files(self, filename): + if self.app.debug: + self.parse_manifest_json() + entry_files = self.manifest.get(filename, {}) + return entry_files.get("js", []) + + def get_css_manifest_files(self, filename): + if self.app.debug: + self.parse_manifest_json() + entry_files = self.manifest.get(filename, {}) + return entry_files.get("css", []) + + @staticmethod + def get_unloaded_chunks(files, loaded_chunks): + filtered_files = [f for f in files if f not in loaded_chunks] + for f in filtered_files: + loaded_chunks.add(f) + return filtered_files + + APP_DIR = os.path.dirname(__file__) appbuilder = AppBuilder(update_perms=False) diff --git a/superset/utils/manifest_processor.py b/superset/utils/manifest_processor.py deleted file mode 100644 index 128cccf1ffb52..0000000000000 --- a/superset/utils/manifest_processor.py +++ /dev/null @@ -1,69 +0,0 @@ -# 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. - -import json - - -class UIManifestProcessor: - def __init__(self, app_dir: str) -> None: - super().__init__() - self.app = None - self.manifest: dict = {} - self.manifest_file = f"{app_dir}/static/assets/dist/manifest.json" - - def init_app(self, app): - self.app = app - # Preload the cache - self.parse_manifest_json() - - @app.context_processor - def get_manifest(): # pylint: disable=unused-variable - return dict( - loaded_chunks=set(), - get_unloaded_chunks=self.get_unloaded_chunks, - js_manifest=self.get_js_manifest_files, - css_manifest=self.get_css_manifest_files, - ) - - def parse_manifest_json(self): - try: - with open(self.manifest_file, "r") as f: - # the manifest includes non-entry files - # we only need entries in templates - full_manifest = json.load(f) - self.manifest = full_manifest.get("entrypoints", {}) - except Exception: # pylint: disable=broad-except - pass - - def get_js_manifest_files(self, filename): - if self.app.debug: - self.parse_manifest_json() - entry_files = self.manifest.get(filename, {}) - return entry_files.get("js", []) - - def get_css_manifest_files(self, filename): - if self.app.debug: - self.parse_manifest_json() - entry_files = self.manifest.get(filename, {}) - return entry_files.get("css", []) - - @staticmethod - def get_unloaded_chunks(files, loaded_chunks): - filtered_files = [f for f in files if f not in loaded_chunks] - for f in filtered_files: - loaded_chunks.add(f) - return filtered_files From ab7a69c7da00de0f145cbd9f3597c8f086bfe37e Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 18 Nov 2019 12:24:28 -0800 Subject: [PATCH 35/42] moving results backend manager around --- superset/extensions.py | 20 ++++++++++++- superset/utils/results_backend_manager.py | 35 ----------------------- 2 files changed, 19 insertions(+), 36 deletions(-) delete mode 100644 superset/utils/results_backend_manager.py diff --git a/superset/extensions.py b/superset/extensions.py index 4e8d0ba026fdc..2974739b71f88 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -25,7 +25,25 @@ from superset.utils.cache_manager import CacheManager from superset.utils.feature_flag_manager import FeatureFlagManager -from superset.utils.results_backend_manager import ResultsBackendManager + + +class ResultsBackendManager: + def __init__(self) -> None: + super().__init__() + self._results_backend = None + self._use_msgpack = False + + def init_app(self, app): + self._results_backend = app.config.get("RESULTS_BACKEND") + self._use_msgpack = app.config.get("RESULTS_BACKEND_USE_MSGPACK") + + @property + def results_backend(self): + return self._results_backend + + @property + def should_use_msgpack(self): + return self._use_msgpack class UIManifestProcessor: diff --git a/superset/utils/results_backend_manager.py b/superset/utils/results_backend_manager.py deleted file mode 100644 index 321f0d51c2c5a..0000000000000 --- a/superset/utils/results_backend_manager.py +++ /dev/null @@ -1,35 +0,0 @@ -# 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. - - -class ResultsBackendManager: - def __init__(self) -> None: - super().__init__() - self._results_backend = None - self._use_msgpack = False - - def init_app(self, app): - self._results_backend = app.config.get("RESULTS_BACKEND") - self._use_msgpack = app.config.get("RESULTS_BACKEND_USE_MSGPACK") - - @property - def results_backend(self): - return self._results_backend - - @property - def should_use_msgpack(self): - return self._use_msgpack From f72438a32513a52ab1d8dee05704aab0daa764f4 Mon Sep 17 00:00:00 2001 From: Craig Date: Mon, 18 Nov 2019 16:15:39 -0800 Subject: [PATCH 36/42] Cleaning up __init__ a bit more --- superset/__init__.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/superset/__init__.py b/superset/__init__.py index e8743c136d462..1de8778ff94b1 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -21,15 +21,15 @@ from superset.app import create_app from superset.connectors.connector_registry import ConnectorRegistry from superset.extensions import ( - appbuilder as ab, + appbuilder, cache_manager, - db as ext_db, - event_logger as ext_event_logger, + db, + event_logger, feature_flag_manager, manifest_processor, results_backend_manager, - security_manager as ext_security_manager, - talisman as ext_talisman, + security_manager, + talisman, ) from superset.security import SupersetSecurityManager from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value @@ -39,11 +39,8 @@ # then initialize it in app.create_app(). These fields will be removed # in subsequent PRs as things are migrated towards the factory pattern app: Flask = current_app -appbuilder = ab cache = LocalProxy(lambda: cache_manager.cache) conf = LocalProxy(lambda: current_app.config) -db = ext_db -event_logger = ext_event_logger get_feature_flags = feature_flag_manager.get_feature_flags get_css_manifest_files = manifest_processor.get_css_manifest_files is_feature_enabled = feature_flag_manager.is_feature_enabled @@ -51,6 +48,4 @@ results_backend_use_msgpack = LocalProxy( lambda: results_backend_manager.should_use_msgpack ) -security_manager = ext_security_manager tables_cache = LocalProxy(lambda: cache_manager.tables_cache) -talisman = ext_talisman From b30ff828893bfb2f6d1ef7c65b27990415fa101a Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 08:28:21 -0800 Subject: [PATCH 37/42] Addressing PR comments --- superset/app.py | 6 ++---- superset/cli.py | 7 ------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/superset/app.py b/superset/app.py index b4f595619ee95..efa0622808a88 100644 --- a/superset/app.py +++ b/superset/app.py @@ -110,8 +110,6 @@ def init_app_in_ctx(self) -> None: self.configure_fab() self.configure_data_sources() - # self.configure_data_sources() - # Hook that provides administrators a handle on the Flask APP # after initialization flask_app_mutator = self.config["FLASK_APP_MUTATOR"] @@ -231,7 +229,7 @@ def __call__(self, environ, start_response): talisman.init_app(self.flask_app, **self.config["TALISMAN_CONFIG"]) def configure_logging(self): - self.flask_app.config.get("LOGGING_CONFIGURATOR").configure_logging( + self.config["LOGGING_CONFIGURATOR"].configure_logging( self.config, self.flask_app.debug ) @@ -246,7 +244,7 @@ def setup_db(self): def configure_wtf(self): if self.config["WTF_CSRF_ENABLED"]: csrf = CSRFProtect(self.flask_app) - csrf_exempt_list = self.config.get("WTF_CSRF_EXEMPT_LIST", []) + csrf_exempt_list = self.config["WTF_CSRF_EXEMPT_LIST"] for ex in csrf_exempt_list: csrf.exempt(ex) diff --git a/superset/cli.py b/superset/cli.py index 138c410e8edeb..905f36a3a9f00 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -34,13 +34,6 @@ from superset.extensions import celery_app, db from superset.utils import core as utils -# from superset import app, appbuilder, db, examples, security_manager, create_app -# from superset.common.tags import add_favorites, add_owners, add_types -# from superset.extensions import celery_app -# from superset.utils import core as utils, dashboard_import_export, dict_import_export - -# config = app.config - @click.group(cls=FlaskGroup, create_app=create_app) @with_appcontext From b333044c0775e1b1541a3e548b570f813ac935a2 Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 08:33:58 -0800 Subject: [PATCH 38/42] Addressing PR comments --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 6680ca6d77674..438ba948ef1a7 100644 --- a/setup.py +++ b/setup.py @@ -110,7 +110,6 @@ def get_git_sha(): extras_require={ "bigquery": ["pybigquery>=0.4.10", "pandas_gbq>=0.10.0"], "cors": ["flask-cors>=2.0.0"], - "flask-testing": ["flask-testing==0.7.1"], "gsheets": ["gsheetsdb>=0.1.9"], "hive": ["pyhive[hive]>=0.6.1", "tableschema", "thrift>=0.11.0, <1.0.0"], "mysql": ["mysqlclient==1.4.2.post1"], @@ -129,4 +128,7 @@ def get_git_sha(): "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", ], + tests_require=[ + "flask-testing==0.7.1", + ], ) From a9ecc7ae047c65e1fa29ac6765584dd6244e47c6 Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 09:48:49 -0800 Subject: [PATCH 39/42] Blacking --- setup.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 438ba948ef1a7..0a3942c6bfed6 100644 --- a/setup.py +++ b/setup.py @@ -128,7 +128,5 @@ def get_git_sha(): "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", ], - tests_require=[ - "flask-testing==0.7.1", - ], + tests_require=["flask-testing==0.7.1"], ) From ccd033802dfdc7eaccfec44763645cde8ce900f3 Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 11:05:57 -0800 Subject: [PATCH 40/42] Fixes for running celery worker --- superset/tasks/__init__.py | 1 - superset/tasks/celery_app.py | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/tasks/__init__.py b/superset/tasks/__init__.py index f6fd1b2b21f9e..fd9417fe5c1e9 100644 --- a/superset/tasks/__init__.py +++ b/superset/tasks/__init__.py @@ -15,4 +15,3 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from . import cache, schedules diff --git a/superset/tasks/celery_app.py b/superset/tasks/celery_app.py index fa172276391ff..90eebd48d2f3d 100644 --- a/superset/tasks/celery_app.py +++ b/superset/tasks/celery_app.py @@ -28,5 +28,8 @@ # Init the Flask app / configure everything create_app() +# Need to import late, as the celery_app will have been setup by "create_app()" +from . import cache, schedules + # Export the celery app globally for Celery (as run on the cmd line) to find app = celery_app From 03a3e4a68698725914afe814928d86abebbd0d10 Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 11:09:22 -0800 Subject: [PATCH 41/42] Tuning isort --- superset/tasks/celery_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/tasks/celery_app.py b/superset/tasks/celery_app.py index 90eebd48d2f3d..bb64b3ab77731 100644 --- a/superset/tasks/celery_app.py +++ b/superset/tasks/celery_app.py @@ -29,7 +29,7 @@ create_app() # Need to import late, as the celery_app will have been setup by "create_app()" -from . import cache, schedules +from . import cache, schedules # isort:skip # Export the celery app globally for Celery (as run on the cmd line) to find app = celery_app From d2f4ada482320a892737d09f75100aa9a3839b0a Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 19 Nov 2019 12:36:04 -0800 Subject: [PATCH 42/42] Blacking --- superset/tasks/celery_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/tasks/celery_app.py b/superset/tasks/celery_app.py index bb64b3ab77731..3724ec72da9e2 100644 --- a/superset/tasks/celery_app.py +++ b/superset/tasks/celery_app.py @@ -29,7 +29,7 @@ create_app() # Need to import late, as the celery_app will have been setup by "create_app()" -from . import cache, schedules # isort:skip +from . import cache, schedules # isort:skip # Export the celery app globally for Celery (as run on the cmd line) to find app = celery_app