Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025

Scattered os.getenv() calls throughout the codebase made configuration hard to understand and maintain. This consolidates all environment variable loading through the existing ConfigManager.

Changes

Extended AppSettings with previously scattered configuration:

  • Config paths: app_config_overrides, app_config_defaults, app_log_dir
  • Runtime settings: environment, runtime_feedback_dir
  • Prompt injection thresholds: pi_threshold_low/medium/high

Replaced direct env access in 8 files:

  • core/capabilities.py, core/otel_config.py, core/prompt_risk.py
  • routes/feedback_routes.py, routes/admin_routes.py, routes/config_routes.py
  • modules/mcp_tools/client.py

Usage pattern:

# Before: scattered throughout codebase
threshold = int(os.getenv("PI_THRESHOLD_LOW", "30"))
log_dir = Path(os.getenv("APP_LOG_DIR") or "logs")

# After: centralized through ConfigManager
from modules.config import config_manager
threshold = config_manager.app_settings.pi_threshold_low
log_dir = Path(config_manager.app_settings.app_log_dir or "logs")

All settings now flow through Pydantic's type-validated AppSettings in modules/config/manager.py. Bootstrap fallbacks preserved in otel_config.py for early initialization.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • astral.sh
    • Triggering command: curl -LsSf REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>centralize the config setup</issue_title>
<issue_description>everything is loaded in one config manager
no loading of random env vars.
easy to understand and configure. </issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Centralize the configuration setup for better clarity Centralize configuration through ConfigManager Nov 4, 2025
Copilot AI requested a review from garland3 November 4, 2025 01:44
Copilot finished work on behalf of garland3 November 4, 2025 01:44
from modules.config import config_manager
if config_manager.app_settings.app_log_dir:
return Path(config_manager.app_settings.app_log_dir)
except Exception:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.

Copilot Autofix

AI 24 days ago

To fix the problem, the except Exception: block should include a logging statement to record that the primary configuration path failed, with the exception details. This ensures that, if there is a problem with the configuration, its nature is not lost, and debugging is easier. The log level should likely be warning, as the program will fallback to a default and continue working, but the failure is noteworthy.

The code change should be in backend/core/otel_config.py, within the _get_logs_dir method. Add a comment explaining the fallback and log the exception (using logging.warning). Since the class handles logging configuration, but may not have been set up yet at this point, basic logging to the root logger is acceptable.

No new imports are needed, as logging is already imported.


Suggested changeset 1
backend/core/otel_config.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/core/otel_config.py b/backend/core/otel_config.py
--- a/backend/core/otel_config.py
+++ b/backend/core/otel_config.py
@@ -90,8 +90,9 @@
             from modules.config import config_manager
             if config_manager.app_settings.app_log_dir:
                 return Path(config_manager.app_settings.app_log_dir)
-        except Exception:            
-            pass
+        except Exception as e:
+            # Could not get log directory from config manager, falling back to default. Log for debugging.
+            logging.warning("Failed to get log directory from config manager, falling back to default: %s", e)
         # Fallback: project_root/logs
         project_root = Path(__file__).resolve().parents[2]
         return project_root / "logs"
EOF
@@ -90,8 +90,9 @@
from modules.config import config_manager
if config_manager.app_settings.app_log_dir:
return Path(config_manager.app_settings.app_log_dir)
except Exception:
pass
except Exception as e:
# Could not get log directory from config manager, falling back to default. Log for debugging.
logging.warning("Failed to get log directory from config manager, falling back to default: %s", e)
# Fallback: project_root/logs
project_root = Path(__file__).resolve().parents[2]
return project_root / "logs"
Copilot is powered by AI and may make mistakes. Always verify output.
@garland3 garland3 marked this pull request as ready for review November 4, 2025 02:40
- Renamed `modules/config/manager.py` to `modules/config/config_manager.py`
- Updated all import statements across the codebase to reflect the new module name
- Added `backend/minio-data/` to `.gitignore` for MinIO data storage
- Minor formatting fix in `otel_config.py` (added space after `except Exception:`)

This change improves module naming clarity and consistency in the config management system.
Copy link
Collaborator

@garland3 garland3 left a comment

Choose a reason for hiding this comment

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

After some additional changes this looks good.

@garland3 garland3 merged commit 1f1d7b7 into main Nov 4, 2025
4 of 5 checks passed
@garland3 garland3 deleted the copilot/centralize-config-setup branch November 4, 2025 02:59
modifying the actual environment or configuration files.
"""

import pytest

Check notice

Code scanning / CodeQL

Unused import Note test

Import of 'pytest' is not used.

Copilot Autofix

AI 24 days ago

To resolve the unused import error, simply remove the line import pytest from the file backend/tests/test_config_manager.py. This will eliminate unnecessary dependency and improve code readability. No other changes are needed, as no features or symbols from pytest are referenced directly in this file.


Suggested changeset 1
backend/tests/test_config_manager.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/tests/test_config_manager.py b/backend/tests/test_config_manager.py
--- a/backend/tests/test_config_manager.py
+++ b/backend/tests/test_config_manager.py
@@ -4,7 +4,6 @@
 modifying the actual environment or configuration files.
 """
 
-import pytest
 from pathlib import Path
 from backend.modules.config.config_manager import (
     ConfigManager,
EOF
@@ -4,7 +4,6 @@
modifying the actual environment or configuration files.
"""

import pytest
from pathlib import Path
from backend.modules.config.config_manager import (
ConfigManager,
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

centralize the config setup

2 participants