From 5efe21bc3552dc01789d0575209b6d6d80255933 Mon Sep 17 00:00:00 2001 From: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> Date: Fri, 13 Sep 2024 15:10:07 +0200 Subject: [PATCH] Use sample config when no config is loaded (#209) --- CHANGELOG | 7 +++++++ zabbix_cli/auth.py | 12 ++++++++++++ zabbix_cli/config/model.py | 2 +- zabbix_cli/logs.py | 17 +++++++++++++++-- zabbix_cli/main.py | 27 +++++++++++++++------------ zabbix_cli/output/console.py | 3 --- zabbix_cli/scripts/init.py | 4 ++-- zabbix_cli/state.py | 24 ++++++++++++++++++++---- zabbix_cli/utils/rich.py | 7 +++++-- 9 files changed, 77 insertions(+), 26 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 89c464e0..b21bb17b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,11 +22,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Now shows a human readable string instead of `0` or `1`. - Example formatting. - Hide defaults for required positional arguments. +- `show_dirs` and `init` no longer requires logging in to the Zabbix API or an existing configuration file. +- Log record format: + - No longer includes the process ID. + - Now includes filename, line number and function name. +- Rich markup is no longer included in log messages. +- Accessing the config when it is not loaded now uses the same sample config as `sample_config` instead of raising an exception. ### Fixed - `show_usermacro_host_list` not showing all hosts with the given macro. - `show_usermacro_template_list` not showing all templates with the given macro. +- Auth token file using username from config file instead of from prompt. ## 3.0.2 diff --git a/zabbix_cli/auth.py b/zabbix_cli/auth.py index 542afbb6..0345671a 100644 --- a/zabbix_cli/auth.py +++ b/zabbix_cli/auth.py @@ -110,6 +110,7 @@ def login(self) -> str: ) token = self.do_login(credentials) logger.info("Logged in with %s", func.__name__) + self.update_config(credentials) return token except ZabbixAPIException as e: logger.warning("Failed to log in with %s: %s", func.__name__, e) @@ -132,6 +133,17 @@ def do_login(self, credentials: Credentials) -> str: auth_token=credentials.auth_token, ) + def update_config(self, credentials: Credentials) -> None: + """Update the config with the provided credentials.""" + from pydantic import SecretStr + + if credentials.username: + self.config.api.username = credentials.username + if credentials.password: + self.config.api.password = SecretStr(credentials.password) + if credentials.auth_token: + self.config.api.auth_token = SecretStr(credentials.auth_token) + def _get_username_password_env(self) -> Credentials: """Get username and password from environment variables.""" return Credentials( diff --git a/zabbix_cli/config/model.py b/zabbix_cli/config/model.py index 74e4a6a5..7bd5327b 100644 --- a/zabbix_cli/config/model.py +++ b/zabbix_cli/config/model.py @@ -243,7 +243,7 @@ class LoggingConfig(BaseModel): # Changed in V3: logging -> enabled (we also allow enable [why?]) validation_alias=AliasChoices("logging", "enabled", "enable"), ) - log_level: LogLevelStr = "ERROR" + log_level: LogLevelStr = "INFO" log_file: Optional[Path] = ( # TODO: define this default path elsewhere LOGS_DIR / "zabbix-cli.log" diff --git a/zabbix_cli/logs.py b/zabbix_cli/logs.py index b1bc7683..3469557f 100644 --- a/zabbix_cli/logs.py +++ b/zabbix_cli/logs.py @@ -38,12 +38,22 @@ DEFAULT_FORMAT = " ".join( ( "%(asctime)s", - "[%(name)s][%(user)s][%(process)d][%(levelname)s]:", + "[%(name)s][%(user)s][%(levelname)s][%(filename)s:%(lineno)d %(funcName)s]:", "%(message)s", ) ) +def remove_markup(text: str) -> str: + """Remove Rich markup from a string.""" + from zabbix_cli.utils.rich import get_text + + # NOTE: we cannot EVER log when removing markup from a record, since + # we would infinitely recurse into this function + t = get_text(text, log=False) + return t.plain + + class ContextFilter(logging.Filter): """Log filter that adds a static field to a record.""" @@ -68,6 +78,7 @@ class SafeFormatter(logging.Formatter): def format(self, record: logging.LogRecord) -> str: record = SafeRecord(record) + record.msg = remove_markup(record.msg) return super().format(record) @@ -117,6 +128,8 @@ def configure_logging(config: LoggingConfig | None = None): from zabbix_cli.config.model import LoggingConfig config = LoggingConfig() + # unconfigured logging uses debug log level to catch everything + config.log_level = "DEBUG" if config.enabled and config.log_file: # log to given filename @@ -135,7 +148,7 @@ def configure_logging(config: LoggingConfig | None = None): root = logging.getLogger() root.handlers.clear() # clear any existing handlers root.addHandler(handler) - root.setLevel(logging.WARNING) + root.setLevel(level) logger.setLevel(level) # configure global app logger # Also log from HTTPX diff --git a/zabbix_cli/main.py b/zabbix_cli/main.py index 2bcab144..de04bc6f 100644 --- a/zabbix_cli/main.py +++ b/zabbix_cli/main.py @@ -34,6 +34,7 @@ from zabbix_cli.app import app from zabbix_cli.config.constants import OutputFormat from zabbix_cli.config.utils import get_config +from zabbix_cli.logs import configure_logging from zabbix_cli.logs import logger if TYPE_CHECKING: @@ -137,12 +138,9 @@ def main_callback( from zabbix_cli.output.console import configure_console from zabbix_cli.state import get_state - if should_skip_configuration(ctx): - return - state = get_state() - if state.is_config_loaded: - conf = state.config + if should_skip_configuration(ctx) or state.is_config_loaded: + conf = state.config # uses a default config else: conf = get_config(config_file) @@ -185,25 +183,30 @@ def main_callback( # TODO: Add a decorator for skipping or some sort of parameter to the existing # StatefulApp.command method that marks a command as not requiring # a configuration file to be loaded. -SKIP_CONFIG_COMMANDS = ["open", "sample_config"] def should_skip_configuration(ctx: typer.Context) -> bool: """Check if the command should skip all configuration of the app.""" - return ctx.invoked_subcommand in SKIP_CONFIG_COMMANDS - - -SKIP_LOGIN_COMMANDS = ["migrate_config"] -# This is a subset of SKIP_CONFIG_COMMANDS, so we don't need to repeat its contents + return ctx.invoked_subcommand in ["open", "sample_config", "show_dirs", "init"] def should_skip_login(ctx: typer.Context) -> bool: """Check if the command should skip logging in to the Zabbix API.""" - return ctx.invoked_subcommand in SKIP_LOGIN_COMMANDS + + if should_skip_configuration(ctx): + return True + return ctx.invoked_subcommand in ["migrate_config"] def main() -> int: """Main entry point for the CLI.""" + # Configure logging with default settings + # We override this later with a custom config in main_callback() + # but we need the basic configuration in order to log to a file without leaking + # logs to stderr for all code that runs _before_ we call configure_logging() + # in main_callback() + configure_logging() + try: app() except Exception as e: diff --git a/zabbix_cli/output/console.py b/zabbix_cli/output/console.py index 2f0e8571..86beec83 100644 --- a/zabbix_cli/output/console.py +++ b/zabbix_cli/output/console.py @@ -141,9 +141,6 @@ def exit_err( Additional keyword arguments to pass to the extra dict. """ state = get_state() - if not state.is_config_loaded: - # HACK: prevents unconfigured logger from printing to stderr - kwargs["log"] = False # Render JSON-formatted error message if output format is JSON if state.is_config_loaded and state.config.app.output_format == "json": diff --git a/zabbix_cli/scripts/init.py b/zabbix_cli/scripts/init.py index 53752ade..8b3f7c10 100644 --- a/zabbix_cli/scripts/init.py +++ b/zabbix_cli/scripts/init.py @@ -13,8 +13,8 @@ @app.callback(invoke_without_command=True) def main_callback( - zabbix_url: str = typer.Option( - "https://zabbix.example.com", + zabbix_url: Optional[str] = typer.Option( + None, "--url", "--zabbix-url", "-z", diff --git a/zabbix_cli/state.py b/zabbix_cli/state.py index cd8cfc22..033ef783 100644 --- a/zabbix_cli/state.py +++ b/zabbix_cli/state.py @@ -26,6 +26,7 @@ # in this one place is convenient. from __future__ import annotations +import logging from typing import TYPE_CHECKING from typing import Any from typing import Optional @@ -41,6 +42,8 @@ from zabbix_cli.config.model import Config from zabbix_cli.pyzabbix.client import ZabbixAPI +logger = logging.getLogger(__name__) + class State: """Object that encapsulates the current state of the application. @@ -66,6 +69,9 @@ def __new__(cls, *args: Any, **kwargs: Any): _config: Optional[Config] = None """Current Config object (may have overrides).""" + _config_loaded: bool = False + """Config has been loaded from file.""" + _config_repl_original: Optional[Config] = None """Config object when the REPL was first launched.""" @@ -101,19 +107,29 @@ def is_client_loaded(self) -> bool: @property def config(self) -> Config: - from zabbix_cli.exceptions import ZabbixCLIError - if self._config is None: - raise ZabbixCLIError("Config not configured") + from zabbix_cli.config.model import Config + from zabbix_cli.logs import configure_logging + + # HACK: configure logging with sample config + # in order to log the debug message to the log file + config = Config.sample_config() + configure_logging(config.logging) + logger.debug( + "Using sample config file as fallback.", + stacklevel=2, # See who called this + ) + return config return self._config @config.setter def config(self, config: Config) -> None: self._config = config + self._config_loaded = True @property def is_config_loaded(self) -> bool: - return self._config is not None + return self._config_loaded @property def console(self) -> Console: diff --git a/zabbix_cli/utils/rich.py b/zabbix_cli/utils/rich.py index e83c8be4..6753c80e 100644 --- a/zabbix_cli/utils/rich.py +++ b/zabbix_cli/utils/rich.py @@ -19,7 +19,7 @@ def get_safe_renderable(renderable: RenderableType) -> RenderableType: return renderable -def get_text(text: str) -> Text: +def get_text(text: str, log: bool = True) -> Text: """Interpret text as markup-styled text, or plain text if it fails.""" try: return Text.from_markup(text) @@ -28,5 +28,8 @@ def get_text(text: str) -> Text: # In most cases, this will be due to some Zabbix item key that looks # like a markup tag, e.g. `system.cpu.load[percpu,avg]` # but we need to log it nonetheless for other cases - logger.debug("Markup error when rendering text: '%s': %s", text, e) + # However, we don't want to log when we're removing markup + # from log records, so we have a `log` parameter to control this. + if log: + logger.debug("Markup error when rendering text: '%s': %s", text, e) return Text(text)