From 4c4cf37f5b0da34dd81f209b7bacdd1ac034a9c6 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sat, 17 Feb 2024 07:24:46 -0500 Subject: [PATCH 1/4] Add option to mangle alert words in retry warnings --- ska_helpers/retry/api.py | 75 +++++++++++++++++++-- ska_helpers/retry/tests/test_retry.py | 94 ++++++++++++++++++++++++++- 2 files changed, 160 insertions(+), 9 deletions(-) diff --git a/ska_helpers/retry/api.py b/ska_helpers/retry/api.py index c4428f0..a6326e3 100644 --- a/ska_helpers/retry/api.py +++ b/ska_helpers/retry/api.py @@ -1,11 +1,13 @@ import functools -import logging import random +import re import sys import time import traceback -logging_logger = logging.getLogger(__name__) +from ska_helpers.logging import basic_logger + +logging_logger = basic_logger(__name__, format="%(message)s") class RetryError(Exception): @@ -21,6 +23,33 @@ def __init__(self, failures): self.failures = failures +def _mangle_alert_words(msg): + """ + Mangle alert words "warning", "error", "fatal", "fail", "exception" in a string. + + This is done by replacing "i" or "l" with "1" and "o" with "0" in the middle of + any of these words. The intent is to avoid triggering the task schedule "check" for + for those words. This is done with a case-insensitive regex substitution. + + Example:: + + >>> mangle_alert_words("WARNING: This is a fatal Error message.") + 'WARN1NG: This is a fata1 Err0r message.' + + :param msg: the string to mangle. + :returns: the mangled string. + """ + for re_word, sub in ( + ("(warn)(i)(ng)", "1"), + ("(err)(o)(r)", "0"), + ("(fata)(l)()", "1"), + ("(fai)(l)()", "1"), + ("(excepti)(o)(n)", "0"), + ): + msg = re.sub(re_word, rf"\g<1>{sub}\3", msg, flags=re.IGNORECASE) + return msg + + def __retry_internal( f, exceptions=Exception, @@ -30,6 +59,7 @@ def __retry_internal( backoff=1, jitter=0, logger=logging_logger, + mangle_alert_words=False, args=None, kwargs=None, ): @@ -46,6 +76,9 @@ def __retry_internal( fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param mangle_alert_words: if True, mangle alert words "warning", "error", "fatal", + "exception", "fail" when issuing a logger warning message. + Default: False. :param args: tuple, function args :param kwargs: dict, function kwargs :returns: the result of the f function. @@ -76,10 +109,13 @@ def __retry_internal( call_args_str = ", ".join(str(arg) for arg in call_args) func_name = getattr(f, "__name__", "func") func_call = f"{func_name}({call_args_str})" - logger.warning( + msg = ( f"WARNING: {func_call} exception: {e}, retrying " f"in {_delay} seconds..." ) + if mangle_alert_words: + msg = _mangle_alert_words(msg) + logger.warning(msg) time.sleep(_delay) _delay *= backoff @@ -101,6 +137,7 @@ def retry( backoff=1, jitter=0, logger=logging_logger, + mangle_alert_words=False, ): """Returns a retry decorator. @@ -113,6 +150,8 @@ def retry( fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param mangle_alert_words: if True, mangle alert words "warning", "error", "fatal", + "exception" when issuing a logger warning message. Default: False. :returns: a retry decorator. """ @@ -128,6 +167,7 @@ def wrapper(*args, **kwargs): backoff, jitter, logger, + mangle_alert_words=mangle_alert_words, args=args, kwargs=kwargs, ) @@ -148,6 +188,7 @@ def retry_call( backoff=1, jitter=0, logger=logging_logger, + mangle_alert_words=False, ): """ Calls a function and re-executes it if it failed. @@ -164,6 +205,9 @@ def retry_call( fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param mangle_alert_words: if True, mangle alert words "warning", "error", "fatal", + "exception", "fail" when issuing a logger warning message. + Default: False. :returns: the result of the f function. """ if args is None: @@ -180,6 +224,7 @@ def retry_call( backoff, jitter, logger, + mangle_alert_words=mangle_alert_words, args=args, kwargs=kwargs, ) @@ -192,7 +237,15 @@ def tables_open_file(*args, **kwargs): it will try again after 2 seconds and once more after 4 seconds. :param *args: args passed through to tables.open_file() - :param **kwargs: kwargs passed through to tables.open_file() + :param mangle_alert_words: (keyword-only) if True, mangle alert words "warning", + "error", "fatal", "exception", "fail" when issuing a logger warning + message. Default: True. + :param retry_delay: (keyword-only) initial delay between attempts. default: 2. + :param retry_tries: (keyword-only) the maximum number of attempts. default: 3. + :param retry_backoff: (keyword-only) multiplier applied to delay between attempts. + default: 2. + :param retry_logger: (keyword-only) logger.warning(msg) will be called. + :param **kwargs: additional kwargs passed through to tables.open_file() :returns: tables file handle """ import tables @@ -200,13 +253,21 @@ def tables_open_file(*args, **kwargs): import ska_helpers.retry + mangle_alert_words = kwargs.pop("mangle_alert_words", True) + retry_delay = kwargs.pop("retry_delay", 2) + retry_tries = kwargs.pop("retry_tries", 3) + retry_backoff = kwargs.pop("retry_backoff", 2) + retry_logger = kwargs.pop("retry_logger", logging_logger) + h5 = ska_helpers.retry.retry_call( tables.open_file, args=args, kwargs=kwargs, exceptions=tables.exceptions.HDF5ExtError, - delay=2, - tries=3, - backoff=2, + delay=retry_delay, + tries=retry_tries, + backoff=retry_backoff, + logger=retry_logger, + mangle_alert_words=mangle_alert_words, ) return h5 diff --git a/ska_helpers/retry/tests/test_retry.py b/ska_helpers/retry/tests/test_retry.py index 31b42c8..05f071c 100644 --- a/ska_helpers/retry/tests/test_retry.py +++ b/ska_helpers/retry/tests/test_retry.py @@ -11,9 +11,10 @@ import time import pytest +import tables -from ska_helpers.retry import RetryError, retry -from ska_helpers.retry.api import retry_call +from ska_helpers.retry import RetryError, retry, tables_open_file +from ska_helpers.retry.api import _mangle_alert_words, retry_call def test_retry(monkeypatch): @@ -212,3 +213,92 @@ def f(value=0): retry_call(f_mock, tries=2) except RetryError as e: assert len(e.failures) == 2 + + +def test_mangle_alert_words(): + # Test case 1: No alert words in the message + msg = "This is a normal message." + assert _mangle_alert_words(msg) == msg + + # Test case 2: Single occurrence of each alert word + msg = "WARNING: Failing fatAL Error exception message." + expected_output = "WARN1NG: Fai1ing fatA1 Err0r excepti0n message." + assert _mangle_alert_words(msg) == expected_output + + # Test case 3: Multiple occurrences of alert words and special characters + msg = "fata1 fata1 fata1WARN1NGWARN1NG!@#$%^&*()" + expected_output = "fata1 fata1 fata1WARN1NGWARN1NG!@#$%^&*()" + assert _mangle_alert_words(msg) == expected_output + + # Test case 4: Empty message + msg = "" + assert _mangle_alert_words(msg) == "" + + +class MockTableOpen: + def __init__(self, n_fail): + self.n_fail = n_fail + + def __call__(self, *args, **kwargs): + msg = """WARNING: Fatal error exception FAILURE: HDF5 error back trace +'/proj/sot/ska3/flight/bin/cheta_update_server_archive --data-root /proj/sot/ska3/flight/data/eng_archive' returned non-zero status: 256 + driver lock request failed + File "H5FDsec2.c", line 1002, in H5FD__sec2_lock + unable to lock file, errno = 11, error message = 'Resource temporarily unavailable' + tables.exceptions.HDF5ExtError: HDF5 error back trace + """ + self.n_fail -= 1 + if self.n_fail >= 0: + raise tables.exceptions.HDF5ExtError(msg) + else: + return "SUCCESS" + + +class MockLogger: + """Mock logger that just prints the message. + + Insanely enough, pytest capsys does not capture logger warnings, not does caplog + capture those warnings. There are years-old GH issues open on this and it seems like + it won't get fixed. After wasting 1/2 hour on this I'm just going to use a print + statement. + """ + + def warning(self, msg): + print(msg) + + +def test_tables_open_file_warning_with_mangle_alert_words(monkeypatch, capsys): + mock_table_open = MockTableOpen(2) + monkeypatch.setattr(tables, "open_file", mock_table_open) + logger = MockLogger() + h5 = tables_open_file("junk.h5", retry_delay=0.01, retry_logger=logger) + assert h5 == "SUCCESS" + out = capsys.readouterr().out.lower() + for word in ["warning", "error", "exception", "fatal", "fail"]: + assert word not in out + for word in ["warn1ng", "err0r", "excepti0n", "fata1", "fai1"]: + assert word in out + + +def test_tables_open_file_warning_without_mangle_alert_words(monkeypatch, capfd): + mock_table_open = MockTableOpen(2) + monkeypatch.setattr(tables, "open_file", mock_table_open) + logger = MockLogger() + h5 = tables_open_file( + "junk.h5", retry_delay=0.01, mangle_alert_words=False, retry_logger=logger + ) + assert h5 == "SUCCESS" + out = capfd.readouterr().out.lower() + for word in ["warning", "error", "exception", "fatal", "fail"]: + assert word in out + for word in ["warn1ng", "err0r", "excepti0n", "fata1", "fai1"]: + assert word not in out + + +def test_tables_open_file_exception_with_mangle_alert_words(monkeypatch): + mock_table_open = MockTableOpen(5) + monkeypatch.setattr(tables, "open_file", mock_table_open) + with pytest.raises(tables.exceptions.HDF5ExtError) as exc: + tables_open_file("junk.h5", retry_delay=0.01) + for word in ["warning", "error", "exception", "fatal", "fail"]: + assert word in str(exc.value).lower() From 5f7363a83299eaec74ae32ebab8f32ea12844a13 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sat, 17 Feb 2024 09:41:25 -0500 Subject: [PATCH 2/4] Update for linting and formatting with ruff --- .github/workflows/black.yml | 10 ---- .github/workflows/flake8.yml | 21 ------- .github/workflows/python-formatting.yml | 10 ++++ .github/workflows/python-linting.yml | 8 +++ .pre-commit-config.yaml | 20 +++---- pyproject.toml | 20 ------- pyrightconfig.json | 8 +++ ruff.toml | 74 +++++++++++++++++++++++++ 8 files changed, 108 insertions(+), 63 deletions(-) delete mode 100644 .github/workflows/black.yml delete mode 100644 .github/workflows/flake8.yml create mode 100644 .github/workflows/python-formatting.yml create mode 100644 .github/workflows/python-linting.yml delete mode 100644 pyproject.toml create mode 100644 pyrightconfig.json create mode 100644 ruff.toml diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml deleted file mode 100644 index 09f2a0a..0000000 --- a/.github/workflows/black.yml +++ /dev/null @@ -1,10 +0,0 @@ -name: Check black formatting - -on: [push] - -jobs: - lint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - uses: psf/black@stable diff --git a/.github/workflows/flake8.yml b/.github/workflows/flake8.yml deleted file mode 100644 index 7fc8102..0000000 --- a/.github/workflows/flake8.yml +++ /dev/null @@ -1,21 +0,0 @@ -name: Python flake8 check - -on: [push] - -jobs: - build: - - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v1 - - name: Set up Python 3.8 - uses: actions/setup-python@v1 - with: - python-version: 3.8 - - name: Lint with flake8 - run: | - pip install flake8 - # Note: only check files in cheta package. Many other files in - # the repo are not maintained as PEP8 compliant. - flake8 . --exclude=docs --count --ignore=W503,W504,F541,E203 --max-line-length=100 --show-source --statistics diff --git a/.github/workflows/python-formatting.yml b/.github/workflows/python-formatting.yml new file mode 100644 index 0000000..4dc79f7 --- /dev/null +++ b/.github/workflows/python-formatting.yml @@ -0,0 +1,10 @@ +name: check format using ruff +on: [push] +jobs: + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: chartboost/ruff-action@v1 + with: + args: format --check diff --git a/.github/workflows/python-linting.yml b/.github/workflows/python-linting.yml new file mode 100644 index 0000000..5b842f0 --- /dev/null +++ b/.github/workflows/python-linting.yml @@ -0,0 +1,8 @@ +name: lint code using ruff +on: [push] +jobs: + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: chartboost/ruff-action@v1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 702f839..f055986 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,13 +1,9 @@ repos: -- repo: https://github.com/psf/black - rev: 23.7.0 - hooks: - - id: black - language_version: python3.10 - -- repo: https://github.com/pycqa/isort - rev: 5.12.0 - hooks: - - id: isort - name: isort (python) - language_version: python3.10 +- repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.1.5 + hooks: + # Run the linter. + - id: ruff + # Run the formatter. + - id: ruff-format diff --git a/pyproject.toml b/pyproject.toml deleted file mode 100644 index 9d60528..0000000 --- a/pyproject.toml +++ /dev/null @@ -1,20 +0,0 @@ -[tool.black] -include = '\.pyi?$' -exclude = ''' -/( - \.git - | \.mypy_cache - | \.tox - | \.venv - | \.vscode - | \.eggs - | _build - | buck-out - | build - | dist - | docs -)/ -''' - -[tool.isort] -profile = "black" diff --git a/pyrightconfig.json b/pyrightconfig.json new file mode 100644 index 0000000..7fef666 --- /dev/null +++ b/pyrightconfig.json @@ -0,0 +1,8 @@ +{ + "ignore": [ + "**/node_modules", + "**/__pycache__", + "**/*.ipynb", + ".git" + ] +} \ No newline at end of file diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..5ea34dc --- /dev/null +++ b/ruff.toml @@ -0,0 +1,74 @@ +# Copied originally from pandas. This config requires ruff >= 0.2. +target-version = "py310" + +# fix = true +lint.unfixable = [] + +lint.select = [ + "I", # isort + "F", # pyflakes + "E", "W", # pycodestyle + "YTT", # flake8-2020 + "B", # flake8-bugbear + "Q", # flake8-quotes + "T10", # flake8-debugger + "INT", # flake8-gettext + "PLC", "PLE", "PLR", "PLW", # pylint + "PIE", # misc lints + "PYI", # flake8-pyi + "TID", # tidy imports + "ISC", # implicit string concatenation + "TCH", # type-checking imports + "C4", # comprehensions + "PGH" # pygrep-hooks +] + +# Some additional rules that are useful +lint.extend-select = [ +"UP009", # UTF-8 encoding declaration is unnecessary +"SIM118", # Use `key in dict` instead of `key in dict.keys()` +"D205", # One blank line required between summary line and description +"ARG001", # Unused function argument +"RSE102", # Unnecessary parentheses on raised exception +"PERF401", # Use a list comprehension to create a transformed list +] + +lint.ignore = [ + "ISC001", # Disable this for compatibility with ruff format + "B028", # No explicit `stacklevel` keyword argument found + "B905", # `zip()` without an explicit `strict=` parameter + "E402", # module level import not at top of file + "E731", # do not assign a lambda expression, use a def + "PLC1901", # compare-to-empty-string + "PLR0911", # Too many returns + "PLR0912", # Too many branches + "PLR0913", # Too many arguments to function call + "PLR0915", # Too many statements + "PLR2004", # Magic number +] + +# TODO : fix these and stop ignoring. Commented out ones are common and OK to except. +lint.extend-ignore = [ + "PGH004", # Use specific rule codes when using `noqa` +# "C401", # Unnecessary generator (rewrite as a `set` comprehension) +# "C402", # Unnecessary generator (rewrite as a dict comprehension) +# "C405", # Unnecessary `list` literal (rewrite as a `set` literal) +# "C408", # Unnecessary `dict` call (rewrite as a literal) +# "C416", # Unnecessary `dict` comprehension (rewrite using `dict()`) +# "G010", # warn is deprecated in favor of warning +# "PYI056", # Calling `.append()` on `__all__` may not be supported by all type checkers +] + +extend-exclude = [ + "docs", +] + +[lint.pycodestyle] +max-line-length = 100 # E501 reports lines that exceed the length of 100. + +[lint.extend-per-file-ignores] +"__init__.py" = ["E402", "F401", "F403"] +# For tests: +# - D205: Don't worry about test docstrings +# - ARG001: Unused function argument false positives for some fixtures +"**/tests/test_*.py" = ["D205", "ARG001"] From 7dc1fc19e067e749e4e81a778393016daef13f20 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sat, 17 Feb 2024 09:49:11 -0500 Subject: [PATCH 3/4] Ignore warnings for a clean ruff (FIX LATER) --- ruff.toml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index 5ea34dc..37be7db 100644 --- a/ruff.toml +++ b/ruff.toml @@ -57,6 +57,18 @@ lint.extend-ignore = [ # "C416", # Unnecessary `dict` comprehension (rewrite using `dict()`) # "G010", # warn is deprecated in favor of warning # "PYI056", # Calling `.append()` on `__all__` may not be supported by all type checkers + # + # These were noted in PR #57 with the transition to ruff and are being ignored only + # to minimize unrelated diffs there. These should mosty be fixed in a subsequent PR. + # + "ARG001", + "PYI041", + "D205", + "I001", + "C403", + "B904", + "B018", + "F541", ] extend-exclude = [ @@ -71,4 +83,4 @@ max-line-length = 100 # E501 reports lines that exceed the length of 100. # For tests: # - D205: Don't worry about test docstrings # - ARG001: Unused function argument false positives for some fixtures -"**/tests/test_*.py" = ["D205", "ARG001"] +"**/tests/test_*.py" = ["D205", "ARG001", "E501"] From c68a9fd46908b9b11765a69047db0318a87e8f0b Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sun, 18 Feb 2024 05:58:57 -0500 Subject: [PATCH 4/4] Update precommit ruff version --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f055986..dabf94a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.1.5 + rev: v0.2.1 hooks: # Run the linter. - id: ruff