Skip to content

feat: add agent file logging and unify logger configuration #43

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.

Sign up for GitHub

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

Open
saksham-jain177 wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −98
Open

feat: add agent file logging and unify logger configuration #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions logger/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
# -*- coding: utf-8 -*-
"""Logger module for GeoAgent plugin."""

from .logger import UILogHandler, get_logger, UILogSignal
from .processing_logger import (
get_processing_logger,
set_processing_ui_log_handler,
)
from .logger import get_logger, configure_logger, attach_ui_handler
from .processing_logger import get_processing_logger

__all__ = [
"UILogHandler",
"get_logger",
"UILogSignal",
"get_processing_logger",
"set_processing_ui_log_handler",
"get_logger",
"configure_logger",
"attach_ui_handler",
"get_processing_logger",
]
100 changes: 42 additions & 58 deletions logger/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
instead of saving to files.
"""

import os
from qgis.core import QgsApplication
import logging
from typing import Optional
from qgis.PyQt.QtWidgets import QTextBrowser
Expand Down Expand Up @@ -40,8 +42,7 @@ class UILogHandler(logging.Handler):
def __init__(
self,
text_browser: Optional[QTextBrowser] = None,
max_lines: int = 1000,
show_debug: bool = False,
max_lines: int = 1000,
):
"""
Initialize UILogHandler.
Expand All @@ -54,7 +55,6 @@ def __init__(
super().__init__()
self.text_browser = text_browser
self.max_lines = max_lines
self.show_debug = show_debug
self.line_count = 0

# Create signal emitter for thread safety
Expand All @@ -77,16 +77,7 @@ def set_text_browser(self, text_browser: QTextBrowser) -> None:
text_browser: QTextBrowser widget to display logs
"""
self.text_browser = text_browser

def set_show_debug(self, show_debug: bool) -> None:
"""
Set whether to show DEBUG level messages.

Args:
show_debug: True to show DEBUG messages, False to hide them
"""
self.show_debug = show_debug

def emit(self, record: logging.LogRecord) -> None:
"""
Emit a log record to the UI.
Expand All @@ -97,10 +88,6 @@ def emit(self, record: logging.LogRecord) -> None:
Args:
record: LogRecord to emit
"""
# Filter out DEBUG messages if not in debug mode
if record.levelno == logging.DEBUG and not self.show_debug:
return

try:
# Format the message
msg = self.format(record)
Expand Down Expand Up @@ -192,51 +179,48 @@ def _trim_lines(self) -> None:
except Exception:
pass

def _get_log_file_path() -> str:
base = QgsApplication.qgisSettingsDirPath()
log_dir = os.path.join(base, "GeoAgent")
os.makedirs(log_dir, exist_ok=True)
return os.path.join(log_dir, "geo_agent.log")

def get_logger(
name: str,
text_browser: Optional[QTextBrowser] = None,
show_debug: bool = False,
level: int = logging.INFO,
) -> logging.Logger:
"""
Get a configured logger instance with UI support.

This function creates or retrieves a logger with both console and UI handlers.

Args:
name: Logger name (typically __name__)
text_browser: Optional QTextBrowser for UI logging
show_debug: Whether to show DEBUG messages (default: False)
level: Logging level (default: logging.INFO)

Returns:
Configured logger instance

Example:
>>> logger = get_logger("GeoAgent.MyModule", text_browser, show_debug=True)
>>> logger.info("This message will appear in both console and UI")
"""
logger = logging.getLogger(name)
def configure_logger(level: int = logging.INFO) -> logging.Logger:
logger = logging.getLogger("geo_agent")
logger.setLevel(level)

# Remove existing handlers to avoid duplicates
logger.handlers.clear()

# Console handler
console_handler = logging.StreamHandler()
console_handler.setLevel(level)
console_formatter = logging.Formatter(
logger.propagate = False
Comment on lines +188 to +191
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger hierarchy/name changed to geo_agent here, but the rest of the plugin still uses logging.getLogger("GeoAgent"/"GeoAgent.UI") and attaches handlers to the root logger (see geo_agent.py). As-is, the new file logger won’t capture those existing logs and UI wiring may not receive geo_agent.* logs once propagate is set to False. Consider aligning on the existing GeoAgent logger name, or configuring/bridging both hierarchies (e.g., attach the file handler to the existing GeoAgent/root logger, or keep geo_agent propagating to root).

Copilot uses AI. Check for mistakes.

# Update existing handlers if present
for h in logger.handlers:
if isinstance(h, logging.FileHandler):
h.setLevel(level)
return logger
Comment on lines +193 to +197
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configure_logger returns as soon as it finds an existing FileHandler, so it won’t update formatter/path (if those ever change) and it also leaves any non-file handlers (e.g., UILogHandler) at their previous levels. Prefer updating all existing handlers’ levels (and formatter where appropriate) instead of returning early.

Copilot uses AI. Check for mistakes.

file_handler = logging.FileHandler(_get_log_file_path(), encoding="utf-8")
file_handler.setLevel(level)

formatter = logging.Formatter(
"%(asctime)s [%(levelname)s] %(name)s: %(message)s",
datefmt="%Y-%m-%d %H:%M:%S",
)
console_handler.setFormatter(console_formatter)
logger.addHandler(console_handler)

# UI handler
if text_browser is not None:
ui_handler = UILogHandler(text_browser, show_debug=show_debug)
ui_handler.setLevel(level)
logger.addHandler(ui_handler)

file_handler.setFormatter(formatter)

logger.addHandler(file_handler)
return logger

def get_logger(name: Optional[str] = None) -> logging.Logger:
if name:
return logging.getLogger(f"geo_agent.{name}")
return logging.getLogger("geo_agent")

def attach_ui_handler(text_browser: QTextBrowser) -> None:
logger = logging.getLogger("geo_agent")

for h in logger.handlers:
if isinstance(h, UILogHandler):
h.set_text_browser(text_browser)
return

ui_handler = UILogHandler(text_browser)
ui_handler.setLevel(logger.level)
logger.addHandler(ui_handler)
35 changes: 5 additions & 30 deletions logger/processing_logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,18 @@
# -*- coding: utf-8 -*-
"""Processing-specific logger helpers for GeoAgent.

This centralizes the processing logger and its UI handler wiring so that
logging concerns stay within the logger package.
Provides a shared processing logger that inherits configuration
from the main GeoAgent logger.
"""

import logging
from typing import Optional
from .logger import get_logger

from ..config.settings import SHOW_DEBUG_LOGS
from .logger import get_logger, UILogHandler


# Create processing logger (no UI handler attached here; UI is wired at runtime)
_processing_logger = get_logger(
"GeoAgent.Processing",
text_browser=None,
show_debug=SHOW_DEBUG_LOGS,
level=logging.DEBUG if SHOW_DEBUG_LOGS else logging.INFO,
)
# Let messages propagate to root; root will carry the UI handler
_processing_logger.propagate = True
# Processing logger inherits handlers and level from core logger
_processing_logger = get_logger("processing")


def get_processing_logger() -> logging.Logger:
"""Return the shared processing logger instance."""
return _processing_logger
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This processing logger now uses get_logger("processing") (i.e., geo_agent.processing), but the plugin still tries to import and call set_processing_ui_log_handler (geo_agent.py:200-202). Since that function was removed, the import fails and UI wiring silently skips processing logs. Either reintroduce a small backwards-compatible set_processing_ui_log_handler shim here (no-op or calls attach_ui_handler/sets propagation), or update the plugin wiring code in the same PR.

Suggested change
return _processing_logger
return _processing_logger
def set_processing_ui_log_handler(handler: logging.Handler) -> None:
"""
Backwards-compatible shim for legacy plugins.
Older plugins expect to call this function to attach a UI log handler
for processing logs. This implementation simply attaches the given
handler to the shared processing logger.
This function is deprecated and may be removed in a future version.
"""
if handler is None:
return
_processing_logger.addHandler(handler)
# Ensure the logger is enabled so handlers receive records
if not _processing_logger.isEnabledFor(logging.INFO):
_processing_logger.setLevel(logging.INFO)

Copilot uses AI. Check for mistakes.


def set_processing_ui_log_handler(ui_log_handler: Optional[UILogHandler]) -> None:
"""Ensure processing logs flow to the shared UI handler via root.

We avoid attaching the UI handler directly to prevent duplicate delivery; the
handler is expected to be registered on the root logger upstream.
"""
global _processing_logger
if ui_log_handler is None:
return

# Remove any direct attachment of this UI handler to avoid duplicates
_processing_logger.handlers = [h for h in _processing_logger.handlers if h is not ui_log_handler]
_processing_logger.propagate = True
Loading