This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9233a63 Event logger config takes instance instead of class (#7997)
9233a63 is described below
commit 9233a63a16311a7fe2012e97a949b9b3fd6434a1
Author: Dave Smith <[email protected]>
AuthorDate: Thu Aug 8 13:47:18 2019 -0700
Event logger config takes instance instead of class (#7997)
* allow preconfigured event logger instance; deprecate specifying class
* add func docs and simplify conditions
* modify docs to reflect EVENT_LOGGER cfg change
* commit black formatting fixes and license header
* add type checking, fix other pre-commit failues
* remove superfluous/wordy condition
* fix flake8 failure
* fix new black failure
* dedent warning msg; use f-strings
---
docs/installation.rst | 4 ++--
superset/__init__.py | 6 ++++--
superset/utils/log.py | 43 +++++++++++++++++++++++++++++++++++++++++++
tests/event_logger_tests.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/docs/installation.rst b/docs/installation.rst
index a012ab9..e560657 100644
--- a/docs/installation.rst
+++ b/docs/installation.rst
@@ -738,9 +738,9 @@ Example of a simple JSON to Stdout class::
print(json.dumps(log))
-Then on Superset's config reference the class you want to use::
+Then on Superset's config pass an instance of the logger type you want to use.
- EVENT_LOGGER = JSONStdOutEventLogger
+ EVENT_LOGGER = JSONStdOutEventLogger()
Upgrading
diff --git a/superset/__init__.py b/superset/__init__.py
index 12d8cf6..3b503a4 100644
--- a/superset/__init__.py
+++ b/superset/__init__.py
@@ -36,7 +36,7 @@ from superset import config
from superset.connectors.connector_registry import ConnectorRegistry
from superset.security import SupersetSecurityManager
from superset.utils.core import pessimistic_connection_handling, setup_cache
-from superset.utils.log import DBEventLogger
+from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value
wtforms_json.init()
@@ -220,7 +220,9 @@ _feature_flags = app.config.get("DEFAULT_FEATURE_FLAGS") or
{}
_feature_flags.update(app.config.get("FEATURE_FLAGS") or {})
# Event Logger
-event_logger = app.config.get("EVENT_LOGGER", DBEventLogger)()
+event_logger = get_event_logger_from_cfg_value(
+ app.config.get("EVENT_LOGGER", DBEventLogger())
+)
def get_feature_flags():
diff --git a/superset/utils/log.py b/superset/utils/log.py
index 94cdfb4..768e6b3 100644
--- a/superset/utils/log.py
+++ b/superset/utils/log.py
@@ -18,7 +18,11 @@
from abc import ABC, abstractmethod
from datetime import datetime
import functools
+import inspect
import json
+import logging
+import textwrap
+from typing import Any, cast, Type
from flask import current_app, g, request
@@ -83,6 +87,45 @@ class AbstractEventLogger(ABC):
return current_app.config.get("STATS_LOGGER")
+def get_event_logger_from_cfg_value(cfg_value: object) -> AbstractEventLogger:
+ """
+ This function implements the deprecation of assignment of class objects to
EVENT_LOGGER
+ configuration, and validates type of configured loggers.
+
+ The motivation for this method is to gracefully deprecate the ability to
configure
+ EVENT_LOGGER with a class type, in favor of preconfigured instances which
may have
+ required construction-time injection of proprietary or locally-defined
dependencies.
+
+ :param cfg_value: The configured EVENT_LOGGER value to be validated
+ :return: if cfg_value is a class type, will return a new instance created
using a
+ default con
+ """
+ result: Any = cfg_value
+ if inspect.isclass(cfg_value):
+ logging.warning(
+ textwrap.dedent(
+ """
+ In superset private config, EVENT_LOGGER has been assigned a
class object. In order to
+ accomodate pre-configured instances without a default
constructor, assignment of a class
+ is deprecated and may no longer work at some point in the
future. Please assign an object
+ instance of a type that implements
superset.utils.log.AbstractEventLogger.
+ """
+ )
+ )
+
+ event_logger_type = cast(Type, cfg_value)
+ result = event_logger_type()
+
+ # Verify that we have a valid logger impl
+ if not isinstance(result, AbstractEventLogger):
+ raise TypeError(
+ "EVENT_LOGGER must be configured with a concrete instance of
superset.utils.log.AbstractEventLogger."
+ )
+
+ logging.info(f"Configured event logger of type {type(result)}")
+ return cast(AbstractEventLogger, result)
+
+
class DBEventLogger(AbstractEventLogger):
def log(self, user_id, action, *args, **kwargs):
from superset.models.core import Log
diff --git a/tests/event_logger_tests.py b/tests/event_logger_tests.py
new file mode 100644
index 0000000..a7a1f22
--- /dev/null
+++ b/tests/event_logger_tests.py
@@ -0,0 +1,44 @@
+# 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 unittest
+
+from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value
+
+
+class TestEventLogger(unittest.TestCase):
+ def test_returns_configured_object_if_correct(self):
+ # test that assignment of concrete AbstractBaseClass impl returns
unmodified object
+ obj = DBEventLogger()
+ res = get_event_logger_from_cfg_value(obj)
+ self.assertTrue(obj is res)
+
+ def test_event_logger_config_class_deprecation(self):
+ # test that assignment of a class object to EVENT_LOGGER is correctly
deprecated
+ res = None
+
+ # print warning if a class is assigned to EVENT_LOGGER
+ with self.assertLogs(level="WARNING"):
+ res = get_event_logger_from_cfg_value(DBEventLogger)
+
+ # class is instantiated and returned
+ self.assertIsInstance(res, DBEventLogger)
+
+ def test_raises_typerror_if_not_abc_impl(self):
+ # test that assignment of non AbstractEventLogger derived type raises
TypeError
+ with self.assertRaises(TypeError):
+ get_event_logger_from_cfg_value(logging.getLogger())
