feat: Lazily resolve config file & load dotenv earlier#1422
feat: Lazily resolve config file & load dotenv earlier#1422edwinjosechittilappilly wants to merge 2 commits intomainfrom
Conversation
Make ConfigManager defer resolving the config file path: store an optional _config_file in __init__ and add a config_file property that lazily imports get_config_file_path and sets the Path on first access. In settings.py, load environment files earlier (load_dotenv and parent dir) and reorder imports so env vars are available before importing config.paths and other config modules; remove duplicate dotenv calls and tidy imports. These changes avoid premature evaluation of paths/configs that depend on environment variables.
Import a module-level logger and add logger.debug calls to each path helper in src/config/paths.py. The change logs the relevant environment variables and computed file paths (including get_config_file_path and get_data_file) to improve visibility when diagnosing configuration issues. No functional behavior of the getters is changed.
github-actions
bot
added
backend
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent premature evaluation of config/path values that depend on environment variables by (a) lazily resolving the config file path in ConfigManager, and (b) loading .env files earlier in settings.py so env vars are available before importing other config modules.
Changes:
- Make
ConfigManagerdefer resolving the config file path until first access via aconfig_fileproperty. - Move
load_dotenv()calls earlier insrc/config/settings.pyto ensure env vars are available sooner. - Add debug logging in
src/config/paths.pyto surface whichOPENRAG_*_PATHenv vars are being used.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/config/settings.py | Reorders imports and loads dotenv earlier to avoid env-dependent config evaluation happening too soon. |
| src/config/paths.py | Adds logging around env-based path helpers to aid debugging of path resolution. |
| src/config/config_manager.py | Introduces lazy config_file resolution to avoid resolving config paths during initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from dotenv import load_dotenv | ||
| load_dotenv(override=False) | ||
| load_dotenv("../", override=False) | ||
|
|
||
| from config.paths import get_flows_path | ||
| from utils.env_utils import get_env_int, get_env_float | ||
|
|
||
| import httpx |
There was a problem hiding this comment.
There are multiple duplicated imports introduced here (e.g., get_env_int/get_env_float and httpx are imported twice). This will trip linters and makes the import ordering harder to reason about; please deduplicate and keep a single import for each symbol/module.
| from dotenv import load_dotenv | |
| load_dotenv(override=False) | |
| load_dotenv("../", override=False) | |
| from config.paths import get_flows_path | |
| from utils.env_utils import get_env_int, get_env_float | |
| import httpx | |
| load_dotenv(override=False) | |
| load_dotenv("../", override=False) | |
| from config.paths import get_flows_path |
| from agentd.patch import patch_openai_with_mcp | ||
| from dotenv import load_dotenv | ||
|
|
||
| from dotenv import load_dotenv |
There was a problem hiding this comment.
load_dotenv is imported twice in a row. Please remove the duplicate import (keep only one) so the module import section stays clean and deterministic.
| from dotenv import load_dotenv |
| logger.debug(f"get_config_file_path: {os.path.join(get_config_path(), 'config.yaml')}") | ||
| return os.path.join(get_config_path(), "config.yaml") |
There was a problem hiding this comment.
get_config_file_path() currently calls get_config_path() twice (once inside the debug f-string and again for the return). That duplicates env lookups and will also emit duplicate debug logs. Please compute the config dir/path once in a local variable and reuse it for both logging and the return value.
| logger.debug(f"get_config_file_path: {os.path.join(get_config_path(), 'config.yaml')}") | |
| return os.path.join(get_config_path(), "config.yaml") | |
| config_path = get_config_path() | |
| logger.debug(f"get_config_file_path: {os.path.join(config_path, 'config.yaml')}") | |
| return os.path.join(config_path, "config.yaml") |
| logger.debug(f"get_data_file: {os.path.join(get_data_path(), filename)}") | ||
| return os.path.join(get_data_path(), filename) |
There was a problem hiding this comment.
get_data_file() calls get_data_path() twice (once for the debug message and once for the return), which repeats env reads and can produce duplicate debug output. Please store data_path = get_data_path() once and reuse it.
| logger.debug(f"get_data_file: {os.path.join(get_data_path(), filename)}") | |
| return os.path.join(get_data_path(), filename) | |
| data_path = get_data_path() | |
| logger.debug(f"get_data_file: {os.path.join(data_path, filename)}") | |
| return os.path.join(data_path, filename) |
Make ConfigManager defer resolving the config file path: store an optional _config_file in init and add a config_file property that lazily imports get_config_file_path and sets the Path on first access. In settings.py, load environment files earlier (load_dotenv and parent dir) and reorder imports so env vars are available before importing config.paths and other config modules; remove duplicate dotenv calls and tidy imports. These changes avoid premature evaluation of paths/configs that depend on environment variables.