Skip to content

Commit

Permalink
Fix logging for cli (#140)
Browse files Browse the repository at this point in the history
* Fix log in cli for cfg; no lock if single threaded

* Rename Log to Logger

* Update changelog
  • Loading branch information
dalmijn authored Nov 7, 2024
1 parent 51ded6f commit 0c608f6
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 45 deletions.
2 changes: 1 addition & 1 deletion docs/_quarto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ quartodoc:
- spawn_logger
- setup_default_log
- setup_mp_log
- name: Log
- name: Logger
children: separate
- name: Receiver
children: separate
Expand Down
4 changes: 4 additions & 0 deletions docs/changelog.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ These are the unreleased changes of Delft-FIAT.
### Added

### Changed
- Disabled locks when running 'single threaded'
- Fixed logging of errors during settings file checks
- Logging class `Log` is now called `Logger`
- Specifying destination ('dst') is now optional for `setup_default_log`

### Deprecated

Expand Down
23 changes: 16 additions & 7 deletions src/fiat/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,32 @@ def info(args):

# Run FIAT function
def run(args):
"""_summary_."""
"""Run the model from cli."""
# Setup the logger, first without a file
logger = setup_default_log(
"fiat",
level=2,
)
sys.stdout.write(fiat_start_str)

# Setup the config reader
cfg = file_path_check(args.config)
cfg = ConfigReader(cfg)
cfg = run_log(ConfigReader, logger, cfg)

# Set the threads is specified
if args.threads is not None:
assert int(args.threads)
cfg.set("global.threads", int(args.threads))

# Setup the logger
# Complete the setup of the logger
loglevel = check_loglevel(cfg.get("global.loglevel", "INFO"))
logger = setup_default_log(
"fiat",
level=loglevel + args.quiet - args.verbose,
logger.add_file_handler(
dst=cfg.get("output.path"),
filename="fiat",
)
sys.stdout.write(fiat_start_str)
logger.level = loglevel

# Add the model version
logger.info(f"Delft-Fiat version: {__version__}")

# Kickstart the model
Expand Down
9 changes: 6 additions & 3 deletions src/fiat/cli/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from pathlib import Path
from typing import Callable

from fiat.log import Log
from fiat.log import Logger


def file_path_check(path):
Expand All @@ -20,11 +20,12 @@ def file_path_check(path):

def run_log(
func: Callable,
logger: Log,
logger: Logger,
*args,
):
"""Cli friendly run for/ with logging exceptions."""
try:
func()
out = func(*args)
except BaseException:
t, v, tb = sys.exc_info()
msg = ",".join([str(item) for item in v.args])
Expand All @@ -33,3 +34,5 @@ def run_log(
logger.error(msg)
# Exit with code 1
sys.exit(1)
else:
return out
63 changes: 33 additions & 30 deletions src/fiat/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ def __init__(
def _check_children(
self,
obj: DummyLog,
logger: "Log",
logger: "Logger",
):
"""Ensure the hierarchy is corrected downwards."""
name = logger.name
Expand All @@ -467,7 +467,7 @@ def _check_children(
logger.parent = child.parent
child.parent = logger

def _check_parents(self, logger: "Log"):
def _check_parents(self, logger: "Logger"):
"""Ensure the hierarchy is corrected upwards."""
name = logger.name
parent = None
Expand All @@ -485,7 +485,7 @@ def _check_parents(self, logger: "Log"):
self.logger_tree[substr] = DummyLog(logger)
else:
obj = self.logger_tree[substr]
if isinstance(obj, Log):
if isinstance(obj, Logger):
parent = obj
break
else:
Expand All @@ -497,7 +497,7 @@ def _check_parents(self, logger: "Log"):

def resolve_logger_tree(
self,
logger: "Log",
logger: "Logger",
):
"""_summary_."""
obj = None
Expand Down Expand Up @@ -556,7 +556,7 @@ def __call__(
obj = cls.__new__(cls, name, level)
cls.__init__(obj, name, level)

res = Log.manager.resolve_logger_tree(obj)
res = Logger.manager.resolve_logger_tree(obj)
if res is not None:
warn(
f"{name} is already in use -> returning currently known object",
Expand Down Expand Up @@ -681,7 +681,7 @@ def start(self):
t.start()


class Log(metaclass=Logmeta):
class Logger(metaclass=Logmeta):
"""Generate a logger.
The list of available logging levels:\n
Expand Down Expand Up @@ -738,7 +738,7 @@ def __del__(self):

def __repr__(self):
_lvl_str = str(LogLevels(self.level)).split(".")[1]
return f"<Log {self.name} level={_lvl_str}>"
return f"<Logger {self.name} level={_lvl_str}>"

def __str__(self):
_mem_loc = f"{id(self):#018x}".upper()
Expand All @@ -758,7 +758,7 @@ def _log(self, record):
else:
obj = obj.parent

def handle_log(log_m):
def _handle_log(log_m):
"""Wrap logging messages."""

def handle(self, *args, **kwargs):
Expand Down Expand Up @@ -808,7 +808,7 @@ def add_file_handler(

@property
def level(self):
"""_summary_."""
"""Return the current logging level."""
return self._level

@level.setter
Expand All @@ -817,40 +817,42 @@ def level(
val: int,
):
self._level = check_loglevel(val)
for h in self._handlers:
h.level = val

def _direct(self, msg):
"""_summary_."""
raise NotImplementedError(NOT_IMPLEMENTED)

@handle_log
@_handle_log
def debug(self, msg: str):
"""Create a debug message."""
return 1, msg

@handle_log
@_handle_log
def info(self, msg: str):
"""Create an info message."""
return 2, msg

@handle_log
@_handle_log
def warning(self, msg: str):
"""Create a warning message."""
return 3, msg

@handle_log
@_handle_log
def error(self, msg: str):
"""Create an error message."""
return 4, msg

@handle_log
@_handle_log
def dead(self, msg: str):
"""Create a kernel-deceased message."""
return 5, msg


def spawn_logger(
name: str,
) -> Log:
) -> Logger:
"""Spawn a logger within a hierarchy.
Parameters
Expand All @@ -860,17 +862,17 @@ def spawn_logger(
Returns
-------
Log
A Log object (for logging).
Logger
A Logger object (for logging).
"""
return Log(name)
return Logger(name)


def setup_default_log(
name: str,
level: int,
dst: str,
) -> Log:
dst: str | None = None,
) -> Logger:
"""Set up the base logger of a hierarchy.
It's advisable to make this a single string that is not concatenated by period.
Expand All @@ -882,25 +884,26 @@ def setup_default_log(
Identifier of the logger.
level : int
Logging level.
dst : str
dst : str | None, optional
The path to where the logging file will be located.
Returns
-------
Log
A Log object (for logging, no really..)
Logger
A Logger object (for logging, no really..)
"""
if len(name.split(".")) > 1:
raise ValueError()
raise ValueError("Only root names (without a period) are allowed.")

obj = Log(name, level=level)
obj = Logger(name, level=level)

obj.add_handler(level=level)
obj.add_file_handler(
dst,
level=level,
filename=name,
)
if dst is not None:
obj.add_file_handler(
dst,
level=level,
filename=name,
)

return obj

Expand Down
5 changes: 3 additions & 2 deletions src/fiat/models/geom.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ def run(

# Setup the jobs
# First setup the locks
lock1 = self._mp_manager.Lock()
lock2 = self._mp_manager.Lock()
lock1, lock2 = (None, None)
if self.threads != 1:
lock1, lock2 = [self._mp_manager.Lock()] * 2
jobs = generate_jobs(
{
"cfg": self.cfg,
Expand Down
4 changes: 2 additions & 2 deletions test/test_log.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import io
from pathlib import Path

from fiat.log import CHandler, Log, MessageFormatter, spawn_logger
from fiat.log import CHandler, Logger, MessageFormatter, spawn_logger


def test_stream(log1, log2):
Expand All @@ -17,7 +17,7 @@ def test_stream(log1, log2):


def test_log(tmp_path):
log = Log("test_log", level=2)
log = Logger("test_log", level=2)
log.add_handler(
level=2,
)
Expand Down

0 comments on commit 0c608f6

Please sign in to comment.