feat: add agent file logging and unify logger configuration#43
feat: add agent file logging and unify logger configuration#43saksham-jain177 wants to merge 1 commit intoiamtekson:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds persistent, file-based logging for GeoAgent under the QGIS profile directory and attempts to centralize logger configuration so agent/processing logs share the same setup.
Changes:
- Introduces
configure_logger()to attach a persistentFileHandlerwriting to<QGIS profile>/GeoAgent/geo_agent.log. - Refactors the processing logger to be derived from the shared “core” logger via
get_logger("processing"). - Updates
loggerpackage exports to surface the new configuration/attachment helpers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
logger/logger.py |
Adds file logging configuration and new logger access/UI attachment helpers; removes show_debug support from UILogHandler. |
logger/processing_logger.py |
Simplifies processing logging to use the shared logger hierarchy. |
logger/__init__.py |
Updates public exports to match the new logger API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 |
There was a problem hiding this comment.
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).
| # Update existing handlers if present | ||
| for h in logger.handlers: | ||
| if isinstance(h, logging.FileHandler): | ||
| h.setLevel(level) | ||
| return logger |
There was a problem hiding this comment.
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.
|
|
||
| def get_processing_logger() -> logging.Logger: | ||
| """Return the shared processing logger instance.""" | ||
| return _processing_logger |
There was a problem hiding this comment.
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.
| 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) |
1 participant
Adding persistent agent log file for GeoAgent
This PR adds file-based logging for GeoAgent under the QGIS profile directory.
Log file location:
/GeoAgent/geo_agent.log
Fixes #3