Skip to content

Commit

Permalink
Revert "Reworking logging. (#384)"
Browse files Browse the repository at this point in the history
This reverts commit c775a21.
  • Loading branch information
JoeZiminski authored Jun 9, 2024
1 parent c775a21 commit 78f04a3
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 108 deletions.
22 changes: 2 additions & 20 deletions datashuttle/utils/ds_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,6 @@
from datashuttle.utils import utils


def get_logger_name():
return "datashuttle"


def get_logger():
return logging.getLogger(get_logger_name())


def logging_is_active():
logger_exists = get_logger_name() in logging.root.manager.loggerDict
if logger_exists and get_logger().handlers != []:
return True
return False


def start(
path_to_log: Path,
command_name: str,
Expand All @@ -53,10 +38,8 @@ def start(
file_log_level="DEBUG",
write_git=True,
log_to_console=False,
logger_name=get_logger_name(),
)
logger = get_logger()
logger.info(f"Starting logging for command {command_name}")
logging.info(f"Starting logging for command {command_name}")


def get_logging_filename(command_name: str) -> str:
Expand Down Expand Up @@ -111,8 +94,7 @@ def close_log_filehandler() -> None:
"""
Remove handlers from all loggers.
"""
logger = get_logger()
logger.debug("Finished logging.")
logger = logging.getLogger()
handlers = logger.handlers[:]
for handler in handlers:
logger.removeHandler(handler)
Expand Down
26 changes: 13 additions & 13 deletions datashuttle/utils/utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
import re
import traceback
import warnings
Expand All @@ -23,9 +24,8 @@ def log(message: str) -> None:
Log the message to the main initialised
logger.
"""
if ds_logger.logging_is_active():
logger = ds_logger.get_logger()
logger.debug(message)
logger = logging.getLogger("datashuttle")
logger.debug(message)


def log_and_message(message: str, use_rich: bool = False) -> None:
Expand All @@ -41,21 +41,13 @@ def log_and_raise_error(message: str, exception: Any) -> None:
"""
Log the message before raising the same message as an error.
"""
if ds_logger.logging_is_active():
logger = ds_logger.get_logger()
if "datashuttle" in logging.root.manager.loggerDict.keys():
logger = logging.getLogger("datashuttle")
logger.error(f"\n\n{' '.join(traceback.format_stack(limit=5))}")
logger.error(message)
raise_error(message, exception)


def warn(message: str, log: bool) -> None:
""" """
if log and ds_logger.logging_is_active():
logger = ds_logger.get_logger()
logger.warning(message)
warnings.warn(message)


def raise_error(message: str, exception) -> None:
"""
Centralized way to raise an error. The logger is closed
Expand All @@ -66,6 +58,14 @@ def raise_error(message: str, exception) -> None:
raise exception(message)


def warn(message: str, log: bool) -> None:
""" """
if log:
logger = logging.getLogger("datashuttle")
logger.warning(message)
warnings.warn(message)


def print_message_to_user(
message: Union[str, list], use_rich: bool = False
) -> None:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies = [
"PyYAML",
"requests",
"rich",
"fancylog>=0.4.2",
"fancylog",
"simplejson",
"pyperclip",
"textual",
Expand Down
2 changes: 1 addition & 1 deletion tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ def set_datashuttle_loggers(disable):
of test_logging.py and turned back off during
tear-down.
"""
for name in [ds_logger.get_logger_name(), "rich"]:
for name in ["datashuttle", "rich"]:
logger = logging.getLogger(name)
logger.disabled = disable

Expand Down
73 changes: 0 additions & 73 deletions tests/tests_integration/test_logging.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import glob
import logging
import os
import re
from pathlib import Path
Expand All @@ -9,85 +8,13 @@

from datashuttle import DataShuttle
from datashuttle.configs.canonical_tags import tags
from datashuttle.utils import ds_logger
from datashuttle.utils.custom_exceptions import (
ConfigError,
NeuroBlueprintError,
)


class TestLogging:

@pytest.fixture(scope="function")
def teardown_logger(self):
"""
Ensure the logger is deleted at the end of each test.
"""
yield
if "datashuttle" in logging.root.manager.loggerDict:
logging.root.manager.loggerDict.pop("datashuttle")

# -------------------------------------------------------------------------
# Basic Functionality Tests
# -------------------------------------------------------------------------

def test_logger_name(self):
"""
Check the canonical logger name.
"""
assert ds_logger.get_logger_name() == "datashuttle"

def test_start_logging(self, tmp_path, teardown_logger):
"""
Test that the central `start` logging function
starts the named logger with the expected handlers.
"""
assert ds_logger.logging_is_active() is False

ds_logger.start(tmp_path, "test-command", variables=[])

# test logger exists and is as expected
assert "datashuttle" in logging.root.manager.loggerDict
assert ds_logger.logging_is_active() is True

logger = logging.getLogger("datashuttle")
assert logger.propagate is False
assert len(logger.handlers) == 1
assert isinstance(logger.handlers[0], logging.FileHandler)

def test_shutdown_logger(self, tmp_path, teardown_logger):
"""
Check the log handler remover indeed removes the handles.
"""
assert ds_logger.logging_is_active() is False

ds_logger.start(tmp_path, "test-command", variables=[])

logger = logging.getLogger("datashuttle")

ds_logger.close_log_filehandler()

assert len(logger.handlers) == 0
assert ds_logger.logging_is_active() is False

def test_logging_an_error(self, project, teardown_logger):
"""
Check that errors are caught and logged properly.
"""
try:
project.create_folders("rawdata", "sob-001")
except:
pass

log = test_utils.read_log_file(project.cfg.logging_path)

assert "ERROR" in log
assert "Problem with name:" in log

# -------------------------------------------------------------------------
# Functional Tests
# -------------------------------------------------------------------------

@pytest.fixture(scope="function")
def clean_project_name(self):
"""
Expand Down

0 comments on commit 78f04a3

Please sign in to comment.