From 7415de149ad64be6a20082ebceb6d1e4b8f2e887 Mon Sep 17 00:00:00 2001 From: BJ Dierkes Date: Tue, 5 Jul 2022 15:20:11 -0500 Subject: [PATCH] Deprecate CEMENT_FRAMEWORK_LOGGING and Hardcoded Debug Option - Resolves Issue #612 (partial) - Resolves Issue #613 - Resolves Issue #631 - Resolves Issue #638 --- CHANGELOG.md | 26 ++++++---- cement/core/deprecations.py | 14 ++++++ cement/core/foundation.py | 56 +++++++++++++++++----- cement/utils/misc.py | 38 ++++++++++----- tests/core/test_deprecations.py | 25 ++++++++++ tests/core/test_foundation.py | 84 ++++++++++++++++++++++++++++++--- 6 files changed, 203 insertions(+), 40 deletions(-) create mode 100644 cement/core/deprecations.py create mode 100644 tests/core/test_deprecations.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f754fc96..1366d7b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,15 @@ Bugs: -- `[cli]` Cement CLI Broken on Python 3.10 +- `[cli]` Cement CLI broken on Python 3.10 - [Issue #619](https://github.com/datafolklabs/cement/issues/619) -- `[cli]` Generated Script Returns Version of Cement +- `[cli]` Generated script returns version of Cement - [Issue #632](https://github.com/datafolklabs/cement/issues/632) -- `[cli]` Generated Script Should Allow Dash/Underscore +- `[cli]` Generated script should allow dash/underscore - [Issue #614](https://github.com/datafolklabs/cement/issues/614) -- `[core.foundation]` App.render() Not Suppressed in Quiet Mode +- `[core.foundation]` App.render() not suppressed in quiet mode - [Issue #636](https://github.com/datafolklabs/cement/issues/636) -- `[core.foundation]` Console Log Not Suppressed by Output Handler Override (JSON/YAML) +- `[core.foundation]` Console log not suppressed by output handler override (JSON/YAML) - [Issue #637](https://github.com/datafolklabs/cement/issues/637) @@ -23,18 +23,26 @@ Features: Refactoring: -- `[utils.misc]` Use SHA256 instead of MD5 in `rando()` to support Redhap/FIPS compliance. Limit to 32 characters for backward compatibility. +- `[utils.misc]` Use SHA256 instead of MD5 in `rando()` to support Redhap/FIPS compliance - [Issue #626](https://github.com/datafolklabs/cement/issues/626) - +- `[core.foundation] Make quiet/debug options configurable + - [Issue #613](https://github.com/datafolklabs/cement/issues/613) Misc: -- `[dev]` Cement CLI Smoke Tests +- `[dev]` Cement CLI smoke tests - [Issue #620](https://github.com/datafolklabs/cement/issues/620) -- `[dev]` Add Python 3.10 to Travis CI tests. +- `[dev]` Add Python 3.10 to Travis CI tests - [Issue #618](https://github.com/datafolklabs/cement/issues/618) +- `[core.deprecations]` Implement Cement deprecation warnings + - [Issue #631](https://github.com/datafolklabs/cement/issues/631) +Deprecations: + +- `[core.foundation]` Deprecate CEMENT_FRAMEWORK_LOGGING in favor of CEMENT_LOG. + - [Issue #638](https://github.com/datafolklabs/cement/issues/638) + ## 3.0.6 - Dec 18, 2021 diff --git a/cement/core/deprecations.py b/cement/core/deprecations.py new file mode 100644 index 00000000..3a809c7f --- /dev/null +++ b/cement/core/deprecations.py @@ -0,0 +1,14 @@ + +from warnings import warn + +DEPRECATIONS = { + '3.0.8-1': "Environment variable CEMENT_FRAMEWORK_LOGGING is deprecated in favor of CEMENT_LOG, and will be removed in Cement v3.2.0", # noqa: E501 + '3.0.8-2': "App.Meta.framework_logging will be changed or removed in Cement v3.2.0", # noqa: E501 +} + + +def deprecate(deprecation_id: str): + deprecation_id = str(deprecation_id) + msg = DEPRECATIONS[deprecation_id] + total_msg = f"{msg}. See: https://docs.builtoncement.com/release-information/deprecations#{deprecation_id}" # noqa: E501 + warn(total_msg, DeprecationWarning, stacklevel=2) diff --git a/cement/core/foundation.py b/cement/core/foundation.py index 6c85d02b..bf231768 100644 --- a/cement/core/foundation.py +++ b/cement/core/foundation.py @@ -11,6 +11,7 @@ from ..core import template from ..core.interface import InterfaceManager from ..core.handler import HandlerManager from ..core.hook import HookManager +from ..core.deprecations import deprecate from ..utils.misc import is_true, minimal_logger from ..utils import fs, misc from ..ext.ext_argparse import ArgparseController as Controller @@ -569,16 +570,21 @@ class App(meta.MetaMixin): framework_logging = True """ - Whether or not to enable Cement framework logging. This is separate - from the application log, and is generally used for debugging issues - with the framework and/or extensions primarily in development. + This setting is deprecated and will be changed or removed in Cement + v3.2.0. See: + https://docs.builtoncement.com/release-information/deprecations#3.0.8-2 + + Whether or not to enable Cement framework logging if ``--debug`` is + passed at the command line. This is separate from the application + log, and is generally used for debugging issues with the framework + and/or extensions primarily in development. This option is overridden by the environment variable - `CEMENT_FRAMEWORK_LOGGING`. Therefore, if in production you do not - want the Cement framework log enabled, you can set this option to - ``False`` but override it in your environment by doing something like - ``export CEMENT_FRAMEWORK_LOGGING=1`` in your shell whenever you need - it enabled. + `CEMENT_LOG`. Therefore, if in production you do not + want the Cement framework log enabled (when the ``debug`` option is + passed at command line), you can set this option to ``False``. Setting + ``CEMENT_LOG=1`` in the environment will trigger this setting to + ``True``. """ define_hooks = [] @@ -738,12 +744,32 @@ class App(meta.MetaMixin): def __init__(self, label=None, **kw): super(App, self).__init__(**kw) - # disable framework logging? - if 'CEMENT_FRAMEWORK_LOGGING' not in os.environ.keys(): - if self._meta.framework_logging is True: - os.environ['CEMENT_FRAMEWORK_LOGGING'] = '1' + # enable framework logging from environment? + if 'CEMENT_LOG' in os.environ.keys(): + val = os.environ.get('CEMENT_LOG') + assert val in ['0', '1'], \ + f'Invalid value for CEMENT_LOG ({val}). Must be one of: 0, 1' + if is_true(val): + self._meta.framework_logging = True else: - os.environ['CEMENT_FRAMEWORK_LOGGING'] = '0' + self._meta.framework_logging = False + + if 'CEMENT_FRAMEWORK_LOGGING' in os.environ.keys(): + deprecate('3.0.8-1') + val = os.environ.get('CEMENT_FRAMEWORK_LOGGING') + assert val in ['0', '1'], ( + f'Invalid value for CEMENT_FRAMEWORK_LOGGING ({val}). Must ' + f'be one of: 0, 1' + ) + if is_true(val): + self._meta.framework_logging = True + else: + self._meta.framework_logging = False + + # DEPRECATE: in v3.2.0, this needs to set os.environ if is True + if self._meta.framework_logging is True: + deprecate('3.0.8-2') + os.environ['CEMENT_LOG_DEPRECATED_DEBUG_OPTION'] = '1' # for convenience we translate this to _meta if label: @@ -787,6 +813,10 @@ class App(meta.MetaMixin): self._lay_cement() + # add deprecation warnings? + # if is_true(os.environ.get('CEMENT_DEPRECATION_WARNINGS', 0)): + # self.hook.register('pre_close', display_deprecation_warnings) + @property def label(self): return self._meta.label diff --git a/cement/utils/misc.py b/cement/utils/misc.py index 50a072ec..c1427bf4 100644 --- a/cement/utils/misc.py +++ b/cement/utils/misc.py @@ -6,6 +6,7 @@ import logging import hashlib from textwrap import TextWrapper from random import random +from ..core.deprecations import deprecate def rando(salt=None): @@ -43,6 +44,7 @@ class MinimalLogger(object): def __init__(self, namespace, debug, *args, **kw): self.namespace = namespace self.backend = logging.getLogger(namespace) + self._debug = debug formatter = logging.Formatter( "%(asctime)s (%(levelname)s) %(namespace)s : %(message)s" ) @@ -51,10 +53,18 @@ class MinimalLogger(object): console.setLevel(logging.INFO) self.backend.setLevel(logging.INFO) - # FIX ME: really don't want to hard check sys.argv like this but - # can't figure any better way to get logging started (only for debug) - # before the app logging is setup. - if '--debug' in sys.argv or debug: + _enabled = False + if is_true(os.environ.get('CEMENT_LOG', 0)) or debug is True: + _enabled = True + elif '--debug' in sys.argv: + deprecate('3.0.8-2') + _enabled = True + elif is_true(os.environ.get('CEMENT_FRAMEWORK_LOGGING', 0)): + deprecate('3.0.8-1') + _enabled = True + + if _enabled is True: + # framework and extensions only log to debug console.setLevel(logging.DEBUG) self.backend.setLevel(logging.DEBUG) @@ -75,15 +85,19 @@ class MinimalLogger(object): @property def logging_is_enabled(self): - if 'CEMENT_FRAMEWORK_LOGGING' in os.environ.keys(): - if is_true(os.environ['CEMENT_FRAMEWORK_LOGGING']): - res = True - else: - res = False - else: - res = True # pragma: nocover + enabled = False + if '--debug' in sys.argv or self._debug: + deprecate('3.0.8-2') + val = os.environ.get('CEMENT_LOG_DEPRECATED_DEBUG_OPTION', 0) + if is_true(val): + enabled = True + elif is_true(os.environ.get('CEMENT_LOG', 0)): + enabled = True + elif is_true(os.environ.get('CEMENT_FRAMEWORK_LOGGING', 0)): + deprecate('3.0.8-1') + enabled = True - return res + return enabled def info(self, msg, namespace=None, **kw): if self.logging_is_enabled: diff --git a/tests/core/test_deprecations.py b/tests/core/test_deprecations.py new file mode 100644 index 00000000..1a2d6043 --- /dev/null +++ b/tests/core/test_deprecations.py @@ -0,0 +1,25 @@ + +import os +import sys +import warnings +from cement.core.foundation import TestApp + + +class TestDeprecations(object): + def test_3_0_8__1(self): + os.environ['CEMENT_FRAMEWORK_LOGGING'] = '1' + + with TestApp() as app: + with warnings.catch_warnings(record=True) as w: + app.run() + assert "3.0.8-1" in str(w[-1].message) + + def test_3_0_8__2(self): + orig_argv = sys.argv + sys.argv = ['--debug'] + + with TestApp() as app: + with warnings.catch_warnings(record=True) as w: + app.run() + assert "3.0.8-2" in str(w[-1].message) + sys.argv = orig_argv diff --git a/tests/core/test_foundation.py b/tests/core/test_foundation.py index 4bcb0652..47266912 100644 --- a/tests/core/test_foundation.py +++ b/tests/core/test_foundation.py @@ -152,20 +152,85 @@ def test_argv(): def test_framework_logging(): # is true - del os.environ['CEMENT_FRAMEWORK_LOGGING'] + env_vars = [ + 'CEMENT_LOG_DEPRECATED_DEBUG_OPTION', + 'CEMENT_LOG', + 'CEMENT_FRAMEWORK_LOGGING' + ] + for var in env_vars: + if var in os.environ.keys(): + del os.environ[var] - with TestApp(framework_logging=True): - assert os.environ['CEMENT_FRAMEWORK_LOGGING'] == '1' + os.environ['CEMENT_LOG'] = '1' + + with TestApp(framework_logging=True) as app: + assert app._meta.framework_logging is True ml = minimal_logger(__name__) assert ml.logging_is_enabled is True # is false - del os.environ['CEMENT_FRAMEWORK_LOGGING'] + orig_argv = sys.argv + sys.argv = ['--debug'] + for var in env_vars: + if var in os.environ.keys(): + del os.environ[var] + os.environ['CEMENT_LOG'] = '0' - with TestApp(framework_logging=False): - assert os.environ['CEMENT_FRAMEWORK_LOGGING'] == '0' + with TestApp() as app: + assert app._meta.framework_logging is False + + ml = minimal_logger(__name__) + assert ml.logging_is_enabled is False + sys.argv = orig_argv + + +def test_framework_logging_deprecated_debug_option(): + # is true + + env_vars = [ + 'CEMENT_LOG_DEPRECATED_DEBUG_OPTION', + 'CEMENT_LOG', + 'CEMENT_FRAMEWORK_LOGGING' + ] + for var in env_vars: + if var in os.environ.keys(): + del os.environ[var] + + orig_argv = sys.argv + sys.argv = ['--debug'] + with TestApp(framework_logging=True): + assert os.environ['CEMENT_LOG_DEPRECATED_DEBUG_OPTION'] == '1' + + ml = minimal_logger(__name__) + assert ml.logging_is_enabled is True + + # is false + + for var in env_vars: + if var in os.environ.keys(): + del os.environ[var] + + ml = minimal_logger(__name__) + assert ml.logging_is_enabled is False + sys.argv = orig_argv + + +def test_deprecated_cement_framework_logging_env_var(): + # is true + + os.environ['CEMENT_FRAMEWORK_LOGGING'] = '1' + + with TestApp() as app: + assert app._meta.framework_logging is True + + ml = minimal_logger(__name__) + assert ml.logging_is_enabled is True + + # is false + + os.environ['CEMENT_FRAMEWORK_LOGGING'] = '0' ml = minimal_logger(__name__) assert ml.logging_is_enabled is False @@ -232,6 +297,13 @@ def test_debug(): with TestApp(argv=['--debug']) as app: assert app.debug is True + # alternative options + argv = ['--not-debug'] + with TestApp(debug_argument_options=['--not-debug']) as app: + assert app.debug is False + with TestApp(debug_argument_options=['--not-debug'], argv=argv) as app: + assert app.debug is True + # config defaults override defaults = init_defaults('my-app-test') defaults['my-app-test']['debug'] = True