Use --config flag everywhere (#127)

Signed-off-by: Damon P. Cortesi <[email protected]>

Author: dacort

src/spark_history_mcp/config/config.py

@@ -1,15 +1,50 @@
 import os
-from typing import Dict, List, Literal, Optional
+from typing import Any, Dict, List, Literal, Optional, Tuple
 
 import yaml
 from pydantic import Field
+from pydantic.fields import FieldInfo
 from pydantic_settings import (
     BaseSettings,
     PydanticBaseSettingsSource,
     SettingsConfigDict,
 )
 
 
+class YamlConfigSettingsSource(PydanticBaseSettingsSource):
+    """Custom settings source that loads configuration from a YAML file.
+
+    The file path is determined by the SHS_MCP_CONFIG environment variable,
+    defaulting to 'config.yaml' if not set.
+    """
+
+    def get_field_value(
+        self, field: FieldInfo, field_name: str
+    ) -> Tuple[Any, str, bool]:
+        # Not used for this implementation
+        return None, field_name, False
+
+    def __call__(self) -> Dict[str, Any]:
+        """Load and return the YAML configuration data."""
+        config_path = os.getenv("SHS_MCP_CONFIG", "config.yaml")
+        is_explicitly_set = "SHS_MCP_CONFIG" in os.environ
+
+        if not os.path.exists(config_path):
+            # If the config file was explicitly specified but doesn't exist, fail fast
+            if is_explicitly_set:
+                raise FileNotFoundError(
+                    f"Config file not found: {config_path}\n"
+                    f"Specified via: SHS_MCP_CONFIG environment variable"
+                )
+            # If using default and it doesn't exist, return empty (will use defaults)
+            return {}
+
+        with open(config_path, "r") as f:
+            config_data = yaml.safe_load(f)
+
+        return config_data or {}
+
+
 class AuthConfig(BaseSettings):
     """Authentication configuration for the Spark server."""
 
@@ -57,17 +92,6 @@ class Config(BaseSettings):
         env_file_encoding="utf-8",
     )
 
-    @classmethod
-    def from_file(cls, file_path: str) -> "Config":
-        """Load configuration from a YAML file."""
-        if not os.path.exists(file_path):
-            return Config()
-
-        with open(file_path, "r") as f:
-            config_data = yaml.safe_load(f)
-
-        return cls.model_validate(config_data)
-
     @classmethod
     def settings_customise_sources(
         cls,
@@ -77,4 +101,16 @@ def settings_customise_sources(
         dotenv_settings: PydanticBaseSettingsSource,
         file_secret_settings: PydanticBaseSettingsSource,
     ) -> tuple[PydanticBaseSettingsSource, ...]:
-        return env_settings, dotenv_settings, init_settings, file_secret_settings
+        # Precedence order (highest to lowest):
+        # 1. Environment variables
+        # 2. .env file
+        # 3. YAML config file (from SHS_MCP_CONFIG)
+        # 4. Init settings (constructor arguments)
+        # 5. File secrets
+        return (
+            env_settings,
+            dotenv_settings,
+            YamlConfigSettingsSource(settings_cls),
+            init_settings,
+            file_secret_settings,
+        )

src/spark_history_mcp/core/app.py

@@ -33,7 +33,8 @@ def default(self, obj):
 
 @asynccontextmanager
 async def app_lifespan(server: FastMCP) -> AsyncIterator[AppContext]:
-    config = Config.from_file("config.yaml")
+    # Config() automatically loads from SHS_MCP_CONFIG env var (set in main.py)
+    config = Config()
 
     clients: dict[str, SparkRestClient] = {}
     default_client = None

src/spark_history_mcp/core/main.py

@@ -30,11 +30,19 @@ def main():
     try:
         logger.info("Starting Spark History Server MCP...")
         logger.info(f"Using config file: {args.config}")
-        config = Config.from_file(args.config)
+
+        # Set the config file path in environment for Pydantic Settings
+        os.environ["SHS_MCP_CONFIG"] = args.config
+
+        # Now Config() will automatically load from the specified YAML file
+        config = Config()
         if config.mcp.debug:
             logger.setLevel(logging.DEBUG)
         logger.debug(json.dumps(json.loads(config.model_dump_json()), indent=4))
         app.run(config)
+    except FileNotFoundError as e:
+        logger.error(f"Configuration error: {e}")
+        sys.exit(1)
     except Exception as e:
         logger.error(f"Failed to start MCP server: {e}")
         sys.exit(1)

tests/emr/test_emr_integration.py

@@ -63,9 +63,9 @@ def test_spark_client_with_emr_session(self, mock_initialize):
         self.assertEqual(apps, [])
 
     @patch("spark_history_mcp.core.app.EMRPersistentUIClient")
-    @patch("spark_history_mcp.core.app.Config.from_file")
+    @patch("spark_history_mcp.core.app.Config")
     def test_app_lifespan_with_emr_config(
-        self, mock_config_from_file, mock_emr_client_class
+        self, mock_config_class, mock_emr_client_class
     ):
         """Test app_lifespan context manager with EMR configuration."""
         import asyncio
@@ -97,7 +97,7 @@ def test_app_lifespan_with_emr_config(
                 emr_cluster_arn=self.emr_cluster_arn, default=True, verify_ssl=True
             )
         }
-        mock_config_from_file.return_value = mock_config
+        mock_config_class.return_value = mock_config
 
         # Use the app_lifespan context manager
         async def test_lifespan():

tests/unit/config.py

@@ -39,8 +39,9 @@ def test_config_from_file(self):
             temp_file_path = temp_file.name
 
         try:
-            # Load config from the file
-            config = Config.from_file(temp_file_path)
+            # Load config from the file using SHS_MCP_CONFIG env var
+            with patch.dict(os.environ, {"SHS_MCP_CONFIG": temp_file_path}):
+                config = Config()
 
             # Verify the loaded configuration
             self.assertEqual(config.mcp.address, "test_host")
@@ -63,9 +64,10 @@ def test_config_from_file(self):
             os.unlink(temp_file_path)
 
     def test_nonexistent_config_file(self):
-        """Test behavior when config file doesn't exist."""
+        """Test behavior when explicitly specified config file doesn't exist."""
         with self.assertRaises(FileNotFoundError):
-            Config.from_file("nonexistent_file.yaml")
+            with patch.dict(os.environ, {"SHS_MCP_CONFIG": "nonexistent_file.yaml"}):
+                Config()
 
     @patch.dict(
         os.environ,
@@ -89,7 +91,8 @@ def test_config_from_env_vars(self):
             temp_file_path = temp_file.name
 
         try:
-            config = Config.from_file(temp_file_path)
+            with patch.dict(os.environ, {"SHS_MCP_CONFIG": temp_file_path}):
+                config = Config()
 
             # Verify MCP config from env vars
             self.assertEqual(config.mcp.address, "env_host")
@@ -122,7 +125,8 @@ def test_env_vars_override_file_config(self):
             temp_file_path = temp_file.name
 
         try:
-            config = Config.from_file(temp_file_path)
+            with patch.dict(os.environ, {"SHS_MCP_CONFIG": temp_file_path}):
+                config = Config()
 
             # Verify that env vars override file config
             self.assertEqual(config.mcp.address, "override_host")
@@ -147,7 +151,8 @@ def test_default_values(self):
             temp_file_path = temp_file.name
 
         try:
-            config = Config.from_file(temp_file_path)
+            with patch.dict(os.environ, {"SHS_MCP_CONFIG": temp_file_path}):
+                config = Config()
 
             # Check MCP defaults
             self.assertEqual(config.mcp.address, "localhost")