Skip to content

Commit

Permalink
Use sample config when no config is loaded (#209)
Browse files Browse the repository at this point in the history
  • Loading branch information
pederhan authored Sep 13, 2024
1 parent fa0e61f commit 5efe21b
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 26 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions zabbix_cli/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion zabbix_cli/config/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
17 changes: 15 additions & 2 deletions zabbix_cli/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand All @@ -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)


Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
27 changes: 15 additions & 12 deletions zabbix_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand Down
3 changes: 0 additions & 3 deletions zabbix_cli/output/console.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
4 changes: 2 additions & 2 deletions zabbix_cli/scripts/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
24 changes: 20 additions & 4 deletions zabbix_cli/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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."""

Expand Down Expand Up @@ -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:
Expand Down
7 changes: 5 additions & 2 deletions zabbix_cli/utils/rich.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

0 comments on commit 5efe21b

Please sign in to comment.