-
Notifications
You must be signed in to change notification settings - Fork 1
fix: honor configured framework runtime settings #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| import stat | ||
| import sys | ||
| import time | ||
| from collections.abc import Awaitable, Callable, Iterable | ||
| from collections.abc import Awaitable, Callable, Iterable, Mapping | ||
| from contextvars import ContextVar, Token | ||
| from dataclasses import dataclass | ||
| from datetime import datetime | ||
|
|
@@ -276,6 +276,41 @@ def _default_log_file(layout: Any, configured_log_file: Path | None) -> Path: | |
| return configured_log_file or layout.log_dir / "primary.log" | ||
|
|
||
|
|
||
| def _parameter_source_was_supplied(source: Any) -> bool: | ||
| """Return whether Click resolved an option from an explicit input source.""" | ||
|
|
||
| return getattr(source, "name", None) in {"COMMANDLINE", "PROMPT", "ENVIRONMENT", "DEFAULT_MAP"} | ||
|
|
||
|
|
||
| def _configured_stream_level( | ||
| configured: str | None, | ||
| *, | ||
| debug: bool, | ||
| quiet: bool, | ||
| debug_source: Any, | ||
| quiet_source: Any, | ||
| ) -> str | None: | ||
| """Merge explicit flag modifiers with the configured user-stream level.""" | ||
|
|
||
| level = configured | ||
| if _parameter_source_was_supplied(debug_source): | ||
| if debug: | ||
| level = "debug" | ||
| elif level == "debug": | ||
| level = "info" | ||
| if _parameter_source_was_supplied(quiet_source) and quiet: | ||
| rank = { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reuse: the |
||
| "debug": logging.DEBUG, | ||
| "info": logging.INFO, | ||
| "warning": logging.WARNING, | ||
| "error": logging.ERROR, | ||
| "critical": logging.CRITICAL, | ||
| } | ||
| if level is None or rank.get(level, logging.INFO) < logging.WARNING: | ||
| level = "warning" | ||
| return level | ||
|
|
||
|
|
||
| def _warn_lifecycle_failure(context: Context[Any, Any, Any], message: str, exc: BaseException) -> None: | ||
| """Report a secondary lifecycle failure without breaking teardown.""" | ||
| try: | ||
|
|
@@ -1015,6 +1050,7 @@ def wrapper(**kwargs: Any) -> Any: | |
| context = self._create_context( | ||
| standard, | ||
| dry_run=resolution.values.dry_run, | ||
| option_sources={key: value.source for key, value in resolution.raw.items()}, | ||
| ) | ||
| except ConfigurationError as exc: | ||
| raise click.UsageError(str(exc)) from exc | ||
|
|
@@ -1110,6 +1146,7 @@ def _create_context( | |
| self, | ||
| standard: dict[str, Any], | ||
| dry_run: bool = False, | ||
| option_sources: Mapping[str, Any] | None = None, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Design risk: |
||
| ) -> Context[dict[str, Any], Any, Any]: | ||
| project = self.profile.discover_project(current_working_dir()) | ||
| manifest_path = project.manifest if project is not None else None | ||
|
|
@@ -1141,10 +1178,28 @@ def _create_context( | |
| or "dev" | ||
| ) | ||
| log_level = framework_config.log_level if framework_config is not None else None | ||
| debug = bool(standard.get("debug") or log_level == "debug") | ||
| sources = option_sources or {} | ||
| debug_source = sources.get("debug") | ||
| quiet_source = sources.get("quiet") | ||
| keep_temp_source = sources.get("keep_temp") | ||
| debug = ( | ||
| bool(standard.get("debug")) | ||
| if _parameter_source_was_supplied(debug_source) or log_level is None | ||
| else log_level == "debug" | ||
| ) | ||
| quiet = bool(standard.get("quiet")) | ||
| keep_temp = bool( | ||
| standard.get("keep_temp") or (framework_config.keep_temp if framework_config is not None else None) | ||
| if framework_config is None or _parameter_source_was_supplied(keep_temp_source): | ||
| keep_temp = bool(standard.get("keep_temp")) | ||
| elif "keep_temp" in config_provenance or framework_config.keep_temp: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cleanup: the |
||
| keep_temp = framework_config.keep_temp | ||
| else: | ||
| keep_temp = bool(standard.get("keep_temp")) | ||
| stream_log_level = _configured_stream_level( | ||
| log_level, | ||
| debug=debug, | ||
| quiet=quiet, | ||
| debug_source=debug_source, | ||
| quiet_source=quiet_source, | ||
| ) | ||
| _capture_effective_output_options( | ||
| owner_app=self, | ||
|
|
@@ -1235,6 +1290,7 @@ def _create_context( | |
| quiet=quiet, | ||
| json_logs=context.json_output, | ||
| run_id=context.run_id, | ||
| log_level=stream_log_level, | ||
| ) | ||
| except OSError as exc: | ||
| target = f"persistent log file '{log_file}'" if log_file is not None else "stderr logging" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,13 @@ | |
| logging.ERROR: "\033[0;31m", | ||
| logging.CRITICAL: "\033[0;31m", | ||
| } | ||
| _CONFIGURED_LOG_LEVELS = { | ||
| "debug": logging.DEBUG, | ||
| "info": logging.INFO, | ||
| "warning": logging.WARNING, | ||
| "error": logging.ERROR, | ||
| "critical": logging.CRITICAL, | ||
| } | ||
|
|
||
|
|
||
| # pylint: disable=too-many-arguments | ||
|
|
@@ -47,7 +54,18 @@ def configure_logger( | |
| formatter: logging.Formatter | None = None, | ||
| json_logs: bool = False, | ||
| run_id: str | None = None, | ||
| log_level: str | None = None, | ||
| ) -> logging.Logger: | ||
| """Configure user-facing and persistent handlers for a CLI logger. | ||
|
|
||
| ``log_level`` optionally selects the user-stream threshold from DEBUG, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc/behavior mismatch: |
||
| INFO, WARNING, ERROR, or CRITICAL. The persistent file handler remains at | ||
| DEBUG. When omitted, the existing ``debug`` and ``quiet`` policy applies. | ||
| """ | ||
| if log_level is not None and log_level not in _CONFIGURED_LOG_LEVELS: | ||
| supported = ", ".join(_CONFIGURED_LOG_LEVELS) | ||
| raise ValueError(f"log_level must be one of: {supported}.") | ||
| stream_level = _user_stream_level(debug, quiet) if log_level is None else _CONFIGURED_LOG_LEVELS[log_level] | ||
| logger = logging.getLogger(f"base_cli.{cli_name}") | ||
| logger.setLevel(logging.DEBUG) | ||
| logger.propagate = False | ||
|
|
@@ -57,7 +75,7 @@ def configure_logger( | |
|
|
||
| user_stream = stream if stream is not None else sys.stderr | ||
| user_handler = logging.StreamHandler(user_stream) | ||
| user_handler.setLevel(_user_stream_level(debug, quiet)) | ||
| user_handler.setLevel(stream_level) | ||
| user_handler.setFormatter( | ||
| _handler_formatter( | ||
| formatter, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reuse:
_parameter_source_was_suppliedhardcodes its own set of ClickParameterSourcenames instead of reusing_parameter_source_rankin_lifecycle_install.py, which this module already imports and which encodes the identical precedence ordering. If Click adds a newParameterSourcevariant, or_parameter_source_rank's ranking is edited, these two independently-maintained checks can silently diverge.