Skip to content

Commit da9f10b

Browse files
ModernMAKModernMAK
authored
Logging improvements for CLI
2 parents 091e3f1 + b42f6c4 commit da9f10b

4 files changed

Lines changed: 262 additions & 45 deletions

File tree

‎src/relic/core/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Core files shared between other Relic-Tool packages.
33
"""
44

5-
__version__ = "2.1.0"
5+
__version__ = "2.2.0"
66

77
from relic.core.cli import CLI
88

‎src/relic/core/cli.py‎

Lines changed: 218 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@
44

55
from __future__ import annotations
66

7+
import argparse
8+
import logging
9+
import os
710
import sys
8-
from argparse import ArgumentParser, Namespace, ArgumentError, Action
11+
from argparse import ArgumentParser, Namespace, ArgumentError
12+
from dataclasses import dataclass
913
from gettext import gettext
14+
from logging.config import fileConfig
15+
from logging.handlers import RotatingFileHandler
1016
from os.path import basename
1117
from typing import (
1218
Optional,
@@ -15,54 +21,210 @@
1521
Any,
1622
Union,
1723
Sequence,
18-
NoReturn,
24+
Callable,
25+
Tuple,
1926
)
2027

21-
from relic.core.errors import UnboundCommandError
28+
from relic.core.errors import UnboundCommandError, RelicArgParserError, RelicArgParser
2229
from relic.core.typeshed import entry_points
2330

31+
LOGLEVEL_TABLE = {
32+
"none": logging.NOTSET,
33+
"debug": logging.DEBUG,
34+
"info": logging.INFO,
35+
"warning": logging.WARNING,
36+
"error": logging.ERROR,
37+
"critical": logging.CRITICAL,
38+
}
2439

25-
class RelicArgParserError(Exception):
26-
"""An error occurred while parsing Command Line arguments"""
2740

41+
def _arg_exists_err(value: str) -> argparse.ArgumentTypeError:
42+
return argparse.ArgumentTypeError(f"The given path '{value}' does not exist!")
2843

29-
def _print_error(parser: ArgumentParser, message: str) -> None:
30-
parser.print_usage(sys.stderr)
31-
args = {"prog": parser.prog, "message": message}
32-
parser.exit(2, gettext("%(prog)s: error: %(message)s\n") % args)
3344

45+
def _get_path_validator(exists: bool) -> Callable[[str], str]:
46+
def _path_type(path: str) -> str:
47+
path = os.path.abspath(path)
3448

35-
class RelicArgParser(ArgumentParser):
36-
"""
37-
Custom ArgParser with special error handling
38-
"""
49+
def _step(_path: str) -> None:
50+
parent, _ = os.path.split(_path)
51+
52+
if len(parent) != 0 and parent != _path:
53+
return _step(parent)
54+
55+
if not os.path.exists(parent):
56+
return None
57+
58+
if os.path.isfile(parent):
59+
raise argparse.ArgumentTypeError(
60+
f"The given path '{path}' is not a valid path; it treats a file ({parent}) as a directory!"
61+
)
3962

40-
def _get_action_from_name(self, name: str | None) -> Action | None:
41-
"""Given a name, get the Action instance registered with this parser.
42-
If only it were made available in the ArgumentError object. It is
43-
passed as it's first arg...
44-
"""
45-
container = self._actions
46-
if name is None:
4763
return None
48-
for action in container:
49-
if "/".join(action.option_strings) == name:
50-
return action
51-
if action.metavar == name:
52-
return action
53-
if action.dest == name:
54-
return action
55-
56-
return None # not found
57-
58-
def error(self, message: str) -> NoReturn:
59-
_, exc, _ = sys.exc_info()
60-
if exc is not None:
61-
if isinstance(exc, ArgumentError) and exc.argument_name is None:
62-
action = self._get_action_from_name(exc.argument_name)
63-
exc.argument_name = action # type:ignore # TODO, investigate
64-
raise exc
65-
raise RelicArgParserError(message)
64+
65+
if exists and not os.path.exists(path):
66+
raise _arg_exists_err(path)
67+
68+
_step(path) # we want step to validate; but we dont care about its result
69+
70+
return path
71+
72+
return _path_type
73+
74+
75+
def _get_dir_type_validator(exists: bool) -> Callable[[str], str]:
76+
validate_path = _get_path_validator(False)
77+
78+
def _dir_type(path: str) -> str:
79+
path = os.path.abspath(path)
80+
if not os.path.exists(path):
81+
if exists:
82+
raise _arg_exists_err(path)
83+
return validate_path(path)
84+
85+
if os.path.isdir(path):
86+
return path
87+
88+
raise argparse.ArgumentTypeError(f"The given path '{path}' is not a directory!")
89+
90+
return _dir_type
91+
92+
93+
def _get_file_type_validator(exists: Optional[bool]) -> Callable[[str], str]:
94+
validate_path = _get_path_validator(False)
95+
96+
def _file_type(path: str) -> str:
97+
path = os.path.abspath(path)
98+
if not os.path.exists(path):
99+
if exists:
100+
raise _arg_exists_err(path)
101+
return validate_path(path)
102+
103+
if os.path.isfile(path):
104+
return path
105+
106+
raise argparse.ArgumentTypeError(f"The given path '{path}' is not a file!")
107+
108+
return _file_type
109+
110+
111+
@dataclass
112+
class LogingOptions:
113+
log_file: Optional[str]
114+
log_level: int
115+
log_config: Optional[str]
116+
117+
118+
def _add_logging_to_parser(
119+
parser: ArgumentParser,
120+
) -> None:
121+
"""Adds [-l --log] and [-ll --loglevel] commands."""
122+
parser.add_argument(
123+
"--log",
124+
type=_get_file_type_validator(False),
125+
help="Path to the log file, if one is generated",
126+
nargs="?",
127+
required=False,
128+
default=None,
129+
)
130+
parser.add_argument(
131+
"--loglevel",
132+
help="Verbosity of the log. Defaults to `info`",
133+
nargs="?",
134+
required=False,
135+
default="info",
136+
choices=list(LOGLEVEL_TABLE.keys()),
137+
)
138+
parser.add_argument(
139+
"--logconfig",
140+
type=_get_file_type_validator(True),
141+
help="Path to a logging config file.",
142+
nargs="?",
143+
required=False,
144+
)
145+
146+
147+
def create_logger_from_namespace(ns: Namespace) -> logging.Logger:
148+
logger = logging.getLogger()
149+
options = _extract_logging_from_namespace(ns)
150+
setup_logging_for_cli(options, logger=logger)
151+
return logger
152+
153+
154+
def _extract_logging_from_namespace(ns: Namespace) -> LogingOptions:
155+
log_file: Optional[str] = ns.log
156+
log_level_name: str = ns.loglevel
157+
log_level = LOGLEVEL_TABLE[log_level_name]
158+
log_config: Optional[str] = ns.logconfig
159+
return LogingOptions(log_file, log_level, log_config)
160+
161+
162+
def _create_log_formatter() -> logging.Formatter:
163+
return logging.Formatter(
164+
fmt="%(levelname)s:%(name)s::%(filename)s:L%(lineno)d:\t%(message)s (%(asctime)s)",
165+
datefmt="%Y-%m-%d %H:%M:%S",
166+
)
167+
168+
169+
def _create_file_handler(log_file: str, log_level: int) -> logging.FileHandler:
170+
f = _create_log_formatter()
171+
h = RotatingFileHandler(
172+
log_file,
173+
encoding="utf8",
174+
maxBytes=100000,
175+
backupCount=-1,
176+
)
177+
h.setFormatter(f)
178+
h.setLevel(log_level)
179+
return h
180+
181+
182+
def _create_console_handlers(
183+
log_level: int, err_level: int = logging.WARNING
184+
) -> Tuple[logging.Handler, logging.Handler]:
185+
f_out = logging.Formatter("%(message)s")
186+
f_err = _create_log_formatter()
187+
188+
h_out = logging.StreamHandler(sys.stdout)
189+
h_err = logging.StreamHandler(sys.stderr)
190+
191+
h_out.setFormatter(f_out)
192+
h_err.setFormatter(f_err)
193+
194+
h_out.addFilter(lambda record: record.levelno < err_level)
195+
h_err.addFilter(lambda record: record.levelno >= err_level)
196+
197+
h_out.setLevel(log_level)
198+
h_err.setLevel(max(err_level, log_level))
199+
return h_out, h_err
200+
201+
202+
def setup_logging_for_cli(
203+
options: LogingOptions,
204+
print_log: bool = True,
205+
logger: Optional[logging.Logger] = None,
206+
) -> None:
207+
logger = logger or logging.getLogger() # Root logger
208+
# Run first to override other loggers
209+
if options.log_config is not None:
210+
fileConfig(options.log_config)
211+
212+
logger.setLevel(options.log_level)
213+
214+
if options.log_file is not None:
215+
h_log_file = _create_file_handler(options.log_file, options.log_level)
216+
logger.addHandler(h_log_file)
217+
218+
if print_log:
219+
h_out, h_err = _create_console_handlers(options.log_level, logging.WARNING)
220+
logger.addHandler(h_out)
221+
logger.addHandler(h_err)
222+
223+
224+
def _print_error(parser: ArgumentParser, message: str) -> None:
225+
parser.print_usage(sys.stderr)
226+
args = {"prog": parser.prog, "message": message}
227+
parser.exit(2, gettext("%(prog)s: error: %(message)s\n") % args)
66228

67229

68230
# Circumvent mypy/pylint shenanigans ~
@@ -138,7 +300,8 @@ def _run(self, ns: Namespace, argv: Optional[Sequence[str]] = None) -> int:
138300
if not hasattr(ns, "function"):
139301
raise UnboundCommandError(cmd)
140302
func = ns.function
141-
result: Optional[int] = func(ns)
303+
logger = create_logger_from_namespace(ns)
304+
result: Optional[int] = func(ns, logger=logger)
142305
if result is None: # Assume success
143306
result = 0
144307
return result
@@ -204,6 +367,7 @@ def __init__(
204367
if self.GROUP is None:
205368
raise ValueError
206369
parser = self._create_parser(parent)
370+
_add_logging_to_parser(parser)
207371
super().__init__(parser)
208372
self.subparsers = self._create_subparser_group(parser)
209373
if load_on_create:
@@ -243,11 +407,13 @@ def load_plugins(self) -> None:
243407
ep_func: CliEntrypoint = ep.load()
244408
ep_func(parent=self.subparsers)
245409

246-
def command(self, ns: Namespace) -> Optional[int]: # pylint: disable=W0613
410+
def command(
411+
self, ns: Namespace, *, logger: logging.Logger
412+
) -> Optional[int]: # pylint: disable=W0613
247413
"""
248414
Adapter which extracts parsed CLI arguments from the namespace and runs the appropriate CLI command
249415
"""
250-
self.parser.print_help(sys.stderr)
416+
logger.info(self.parser.format_help())
251417
return 1
252418

253419

@@ -263,6 +429,7 @@ class CliPlugin(_CliPlugin): # pylint: disable= too-few-public-methods
263429

264430
def __init__(self, parent: Optional[_SubParsersAction] = None):
265431
parser = self._create_parser(parent)
432+
_add_logging_to_parser(parser)
266433
super().__init__(parser)
267434
if self.parser.get_default("function") is None:
268435
self.parser.set_defaults(function=self.command)
@@ -309,3 +476,13 @@ def _create_parser(
309476

310477
if __name__ == "__main__":
311478
CLI.run()
479+
480+
__all__ = [
481+
"RelicArgParserError", # Should move to relic.core.errors in next major
482+
"RelicArgParser", # Should move to relic.core.errors in next major
483+
"CLI",
484+
"CliPlugin",
485+
"CliPluginGroup",
486+
"CliEntrypoint",
487+
"RelicCli",
488+
]

‎src/relic/core/errors.py‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
from __future__ import annotations
66

7-
from typing import Any, Optional, TypeVar, Generic
7+
import sys
8+
from argparse import ArgumentParser, Action, ArgumentError
9+
10+
from typing import Any, Optional, TypeVar, Generic, NoReturn
811

912

1013
def _print_mismatch(name: str, received: Optional[Any], expected: Optional[Any]) -> str:
@@ -133,3 +136,40 @@ def __init__(
133136
"RelicSerializationError",
134137
"RelicSerializationSizeError",
135138
]
139+
140+
141+
class RelicArgParserError(Exception):
142+
"""An error occurred while parsing Command Line arguments"""
143+
144+
145+
class RelicArgParser(ArgumentParser):
146+
"""
147+
Custom ArgParser with special error handling
148+
"""
149+
150+
def _get_action_from_name(self, name: str | None) -> Action | None:
151+
"""Given a name, get the Action instance registered with this parser.
152+
If only it were made available in the ArgumentError object. It is
153+
passed as it's first arg...
154+
"""
155+
container = self._actions
156+
if name is None:
157+
return None
158+
for action in container:
159+
if "/".join(action.option_strings) == name:
160+
return action
161+
if action.metavar == name:
162+
return action
163+
if action.dest == name:
164+
return action
165+
166+
return None # not found
167+
168+
def error(self, message: str) -> NoReturn:
169+
_, exc, _ = sys.exc_info()
170+
if exc is not None:
171+
if isinstance(exc, ArgumentError) and exc.argument_name is None:
172+
action = self._get_action_from_name(exc.argument_name)
173+
exc.argument_name = action # type:ignore # TODO, investigate
174+
raise exc
175+
raise RelicArgParserError(message)

‎tests/test_cli.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ def _ArgumentError(name: str, message: str):
6969
return _
7070

7171

72-
_HELP = ["relic", "-h"], """usage: relic [-h] {} ...""", 0, True
73-
_NO_SUB_CMD = ["relic"], """usage: relic [-h] {} ...""", 1, False
72+
_HELP = ["relic", "-h"], """usage: relic [-h]""", 0, True
73+
_NO_SUB_CMD = ["relic"], """usage: relic [-h]""", 1, True
7474
_T = """relic: error: argument command: invalid choice: '{name}' (choose from )"""
7575
_BAD_SUB_CMD = (
7676
["relic", "Paul_Chambers"],

0 commit comments

Comments
 (0)