🐛 Avoid fancy logs for non-TTY output

Shortcake-Parent: main

Author: patrick91

src/fastapi_cli/cli.py

@@ -20,7 +20,11 @@
 
 from . import __version__
 from .logging import setup_logging
-from .utils.cli import get_rich_toolkit, get_uvicorn_log_config
+from .utils.cli import (
+    get_rich_toolkit,
+    get_uvicorn_log_config,
+    should_use_rich_logs,
+)
 
 app = typer.Typer(
     rich_markup_mode="rich", context_settings={"help_option_names": ["-h", "--help"]}
@@ -271,6 +275,10 @@ def _run(
         toolkit.print("Logs:")
         toolkit.print_line()
 
+        extra_uvicorn_kwargs: dict[str, Any] = {}
+        if should_use_rich_logs():
+            extra_uvicorn_kwargs["log_config"] = get_uvicorn_log_config()
+
         uvicorn.run(
             app=import_string,
             host=host,
@@ -285,7 +293,7 @@ def _run(
             root_path=root_path,
             proxy_headers=proxy_headers,
             forwarded_allow_ips=forwarded_allow_ips,
-            log_config=get_uvicorn_log_config(),
+            **extra_uvicorn_kwargs,
         )
 
 

src/fastapi_cli/logging.py

@@ -1,22 +1,38 @@
 import logging
 
-from rich.console import Console
+from rich.errors import MarkupError
 from rich.logging import RichHandler
+from rich.markup import render
 
+from .utils.cli import should_use_rich_logs
 
-def setup_logging(terminal_width: int | None = None, level: int = logging.INFO) -> None:
+
+class PlainFormatter(logging.Formatter):
+    def formatMessage(self, record: logging.LogRecord) -> str:
+        try:
+            record.message = render(record.message, emoji=False).plain
+        except MarkupError:
+            pass
+        return super().formatMessage(record)
+
+
+def setup_logging(level: int = logging.INFO) -> None:
     logger = logging.getLogger("fastapi_cli")
-    console = Console(width=terminal_width) if terminal_width else None
-    rich_handler = RichHandler(
-        show_time=False,
-        rich_tracebacks=True,
-        tracebacks_show_locals=True,
-        markup=True,
-        show_path=False,
-        console=console,
-    )
-    rich_handler.setFormatter(logging.Formatter("%(message)s"))
-    logger.addHandler(rich_handler)
+    if should_use_rich_logs():
+        rich_handler = RichHandler(
+            show_time=False,
+            rich_tracebacks=True,
+            tracebacks_show_locals=True,
+            markup=True,
+            show_path=False,
+        )
+        rich_handler.setFormatter(logging.Formatter("%(message)s"))
+        handler: logging.Handler = rich_handler
+    else:
+        handler = logging.StreamHandler()
+        handler.setFormatter(PlainFormatter("%(levelname)s:%(name)s:%(message)s"))
+
+    logger.addHandler(handler)
 
     logger.setLevel(level)
     logger.propagate = False

src/fastapi_cli/utils/cli.py

@@ -1,4 +1,5 @@
 import logging
+import sys
 from typing import Any
 
 from rich_toolkit import RichToolkit, RichToolkitTheme
@@ -20,6 +21,10 @@ def formatMessage(self, record: logging.LogRecord) -> str:
         return result
 
 
+def should_use_rich_logs() -> bool:
+    return sys.stdout.isatty()
+
+
 def get_uvicorn_log_config() -> dict[str, Any]:
     return {
         "version": 1,

tests/conftest.py

@@ -20,5 +20,5 @@ def reset_syspath() -> Generator[None, None, None]:
 def setup_terminal() -> None:
     rich_utils.MAX_WIDTH = 3000
     rich_utils.FORCE_TERMINAL = False
-    setup_logging(terminal_width=3000)
+    setup_logging()
     return

tests/test_cli.py

@@ -16,6 +16,11 @@
 assets_path = Path(__file__).parent / "assets"
 
 
+@pytest.fixture(autouse=True)
+def force_rich_logs(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr("fastapi_cli.cli.should_use_rich_logs", lambda: True)
+
+
 def test_dev() -> None:
     with changing_dir(assets_path):
         with patch.object(uvicorn, "run") as mock_run:
@@ -51,6 +56,21 @@ def test_dev() -> None:
         assert "🐍 single_file_app.py" in result.output
 
 
+def test_run_uses_uvicorn_default_log_config_without_rich_logs(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr("fastapi_cli.cli.should_use_rich_logs", lambda: False)
+
+    with changing_dir(assets_path):
+        with patch.object(uvicorn, "run") as mock_run:
+            result = runner.invoke(app, ["run", "single_file_app.py"])
+            assert result.exit_code == 0, result.output
+            assert mock_run.called
+            assert mock_run.call_args
+
+    assert "log_config" not in mock_run.call_args.kwargs
+
+
 def test_dev_no_args_auto_discovery() -> None:
     """Test that auto-discovery works when no args and no pyproject.toml entrypoint"""
     with changing_dir(assets_path / "default_files" / "default_main"):

tests/test_utils_cli.py

@@ -1,9 +1,18 @@
+import io
 import logging
+import sys
 from logging.config import dictConfig
 
-from pytest import LogCaptureFixture
+from pytest import LogCaptureFixture, MonkeyPatch
+from rich.logging import RichHandler
 
-from fastapi_cli.utils.cli import CustomFormatter, get_uvicorn_log_config
+from fastapi_cli import logging as cli_logging
+from fastapi_cli.logging import PlainFormatter
+from fastapi_cli.utils.cli import (
+    CustomFormatter,
+    get_uvicorn_log_config,
+    should_use_rich_logs,
+)
 
 
 def test_get_uvicorn_config_uses_custom_formatter() -> None:
@@ -14,6 +23,68 @@ def test_get_uvicorn_config_uses_custom_formatter() -> None:
     assert config["loggers"]["uvicorn"]["propagate"] is False
 
 
+def test_should_use_rich_logs_is_false_without_tty(
+    monkeypatch: MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(sys, "stdout", io.StringIO())
+
+    assert should_use_rich_logs() is False
+
+
+def test_plain_formatter_strips_rich_markup() -> None:
+    formatter = PlainFormatter("%(levelname)s:%(name)s:%(message)s")
+    record = logging.LogRecord(
+        name="fastapi_cli.discover",
+        level=logging.WARNING,
+        pathname="",
+        lineno=0,
+        msg="Ensure all the package directories have an [blue]__init__.py[/blue] file",
+        args=(),
+        exc_info=None,
+    )
+
+    assert (
+        formatter.format(record)
+        == "WARNING:fastapi_cli.discover:Ensure all the package directories have an __init__.py file"
+    )
+
+
+def test_plain_formatter_preserves_invalid_markup() -> None:
+    formatter = PlainFormatter("%(message)s")
+    record = logging.LogRecord(
+        name="fastapi_cli.discover",
+        level=logging.WARNING,
+        pathname="",
+        lineno=0,
+        msg="[/blue]",
+        args=(),
+        exc_info=None,
+    )
+
+    assert formatter.format(record) == "[/blue]"
+
+
+def test_setup_logging_uses_rich_handler_when_enabled(
+    monkeypatch: MonkeyPatch,
+) -> None:
+    logger = logging.getLogger("fastapi_cli")
+    original_handlers = logger.handlers.copy()
+    original_level = logger.level
+    original_propagate = logger.propagate
+    logger.handlers.clear()
+    monkeypatch.setattr(cli_logging, "should_use_rich_logs", lambda: True)
+    try:
+        cli_logging.setup_logging(level=logging.DEBUG)
+
+        assert isinstance(logger.handlers[0], RichHandler)
+        assert logger.level == logging.DEBUG
+        assert logger.propagate is False
+    finally:
+        logger.handlers = original_handlers
+        logger.setLevel(original_level)
+        logger.propagate = original_propagate
+
+
 def test_custom_formatter() -> None:
     formatter = CustomFormatter()
 

tests/test_utils_package.py

@@ -1,10 +1,12 @@
+import io
+import logging
 from pathlib import Path
 
 import pytest
-from pytest import CaptureFixture
 
 from fastapi_cli.discover import get_import_data
 from fastapi_cli.exceptions import FastAPICLIException
+from fastapi_cli.logging import PlainFormatter
 from tests.utils import changing_dir
 
 assets_path = Path(__file__).parent / "assets"
@@ -158,15 +160,24 @@ def test_package_dir_explicit_app() -> None:
     assert import_data.module_data.module_import_str == "package"
 
 
-def test_broken_package_dir(capsys: CaptureFixture[str]) -> None:
-    with changing_dir(assets_path):
-        # TODO (when deprecating Python 3.8): remove ValueError
-        with pytest.raises((ImportError, ValueError)):
-            get_import_data(path=Path("broken_package/mod/app.py"))
-
-    captured = capsys.readouterr()
-    assert "Import error:" in captured.out
-    assert "Ensure all the package directories have an __init__.py file" in captured.out
+def test_broken_package_dir() -> None:
+    stream = io.StringIO()
+    handler = logging.StreamHandler(stream)
+    handler.setFormatter(PlainFormatter("%(levelname)s:%(name)s:%(message)s"))
+    logger = logging.getLogger("fastapi_cli")
+    logger.addHandler(handler)
+    try:
+        with changing_dir(assets_path):
+            # TODO (when deprecating Python 3.8): remove ValueError
+            with pytest.raises((ImportError, ValueError)):
+                get_import_data(path=Path("broken_package/mod/app.py"))
+    finally:
+        logger.removeHandler(handler)
+        handler.close()
+
+    output = stream.getvalue()
+    assert "Import error:" in output
+    assert "Ensure all the package directories have an __init__.py file" in output
 
 
 def test_package_dir_no_app() -> None: