OTAVA-82: use ConfigArgParse to create Config

otava/bigquery.py

@@ -32,6 +32,20 @@ class BigQueryConfig:
     dataset: str
     credentials: str
 
+    @staticmethod
+    def add_parser_args(arg_group):
+        arg_group.add_argument("--bigquery-project-id", help="BigQuery project ID", env_var="BIGQUERY_PROJECT_ID")
+        arg_group.add_argument("--bigquery-dataset", help="BigQuery dataset", env_var="BIGQUERY_DATASET")
+        arg_group.add_argument("--bigquery-credentials", help="BigQuery credentials file", env_var="BIGQUERY_VAULT_SECRET")
+
+    @staticmethod
+    def from_parser_args(args):
+        return BigQueryConfig(
+            project_id=getattr(args, 'bigquery_project_id', None),
+            dataset=getattr(args, 'bigquery_dataset', None),
+            credentials=getattr(args, 'bigquery_credentials', None)
+        )
+
 
 @dataclass
 class BigQueryError(Exception):

otava/config.py

@@ -14,13 +14,12 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-
-import os
+import logging
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Dict, List, Optional
 
-from expandvars import expandvars
+import configargparse
 from ruamel.yaml import YAML
 
 from otava.bigquery import BigQueryConfig
@@ -96,107 +95,90 @@ def load_test_groups(config: Dict, tests: Dict[str, TestConfig]) -> Dict[str, Li
     return result
 
 
-def load_config_from(config_file: Path) -> Config:
-    """Loads config from the specified location"""
-    try:
-        content = expandvars(config_file.read_text(), nounset=True)
+def load_config_from_parser_args(args: configargparse.Namespace) -> Config:
+    config_file = getattr(args, "config_file", None)
+    if config_file is not None:
         yaml = YAML(typ="safe")
-        config = yaml.load(content)
-        """
-        if Grafana configs not explicitly set in yaml file, default to same as Graphite
-        server at port 3000
-        """
-        graphite_config = None
-        grafana_config = None
-        if "graphite" in config:
-            if "url" not in config["graphite"]:
-                raise ValueError("graphite.url")
-            graphite_config = GraphiteConfig(url=config["graphite"]["url"])
-            if config.get("grafana") is None:
-                config["grafana"] = {}
-                config["grafana"]["url"] = f"{config['graphite']['url'].strip('/')}:3000/"
-                config["grafana"]["user"] = os.environ.get("GRAFANA_USER", "admin")
-                config["grafana"]["password"] = os.environ.get("GRAFANA_PASSWORD", "admin")
-            grafana_config = GrafanaConfig(
-                url=config["grafana"]["url"],
-                user=config["grafana"]["user"],
-                password=config["grafana"]["password"],
-            )
-
-        slack_config = None
-        if config.get("slack") is not None:
-            if not config["slack"]["token"]:
-                raise ValueError("slack.token")
-            slack_config = SlackConfig(
-                bot_token=config["slack"]["token"],
-            )
-
-        postgres_config = None
-        if config.get("postgres") is not None:
-            if not config["postgres"]["hostname"]:
-                raise ValueError("postgres.hostname")
-            if not config["postgres"]["port"]:
-                raise ValueError("postgres.port")
-            if not config["postgres"]["username"]:
-                raise ValueError("postgres.username")
-            if not config["postgres"]["password"]:
-                raise ValueError("postgres.password")
-            if not config["postgres"]["database"]:
-                raise ValueError("postgres.database")
-
-            postgres_config = PostgresConfig(
-                hostname=config["postgres"]["hostname"],
-                port=config["postgres"]["port"],
-                username=config["postgres"]["username"],
-                password=config["postgres"]["password"],
-                database=config["postgres"]["database"],
-            )
-
-        bigquery_config = None
-        if config.get("bigquery") is not None:
-            bigquery_config = BigQueryConfig(
-                project_id=config["bigquery"]["project_id"],
-                dataset=config["bigquery"]["dataset"],
-                credentials=config["bigquery"]["credentials"],
-            )
+        config = yaml.load(Path(config_file).read_text())
 
         templates = load_templates(config)
         tests = load_tests(config, templates)
         groups = load_test_groups(config, tests)
-
-        return Config(
-            graphite=graphite_config,
-            grafana=grafana_config,
-            slack=slack_config,
-            postgres=postgres_config,
-            bigquery=bigquery_config,
-            tests=tests,
-            test_groups=groups,
-        )
-
-    except FileNotFoundError as e:
-        raise ConfigError(f"Configuration file not found: {e.filename}")
-    except KeyError as e:
-        raise ConfigError(f"Configuration key not found: {e.args[0]}")
-    except ValueError as e:
-        raise ConfigError(f"Value for configuration key not found: {e.args[0]}")
-
-
-def load_config() -> Config:
-    """Loads config from one of the default locations"""
-
-    env_config_path = os.environ.get("OTAVA_CONFIG")
-    if env_config_path:
-        return load_config_from(Path(env_config_path).absolute())
-
-    paths = [
-        Path().home() / ".otava/otava.yaml",
-        Path().home() / ".otava/conf.yaml",
-        Path(os.path.realpath(__file__)).parent / "resources/otava.yaml",
-    ]
-
-    for p in paths:
-        if p.exists():
-            return load_config_from(p)
-
-    raise ConfigError(f"No configuration file found. Checked $OTAVA_CONFIG and searched: {paths}")
+    else:
+        logging.warning("Otava configuration file not found or not specified")
+        tests = {}
+        groups = {}
+
+    return Config(
+        graphite=GraphiteConfig.from_parser_args(args),
+        grafana=GrafanaConfig.from_parser_args(args),
+        slack=SlackConfig.from_parser_args(args),
+        postgres=PostgresConfig.from_parser_args(args),
+        bigquery=BigQueryConfig.from_parser_args(args),
+        tests=tests,
+        test_groups=groups,
+    )
+
+
+class NestedYAMLConfigFileParser(configargparse.ConfigFileParser):
+    """
+    Custom YAML config file parser that supports nested YAML structures.
+    Maps nested keys like 'slack: {token: value}' to 'slack-token=value', i.e. CLI argument style.
+    Recasts values from YAML inferred types to strings as expected for CLI arguments.
+    """
+
+    def parse(self, stream):
+        yaml = YAML(typ="safe")
+        config_data = yaml.load(stream)
+        if config_data is None:
+            return {}
+        flattened_dict = {}
+        self._flatten_dict(config_data, flattened_dict)
+        return flattened_dict
+
+    def _flatten_dict(self, nested_dict, flattened_dict, prefix=''):
+        """Recursively flatten nested dictionaries using CLI dash-separated notation for keys."""
+        if not isinstance(nested_dict, dict):
+            return
+
+        for key, value in nested_dict.items():
+            new_key = f"{prefix}{key}" if prefix else key
+
+            # yaml keys typically use snake case
+            # replace underscore with dash to convert snake case to CLI dash-separated style
+            new_key = new_key.replace("_", "-")
+
+            if isinstance(value, dict):
+                # Recursively process nested dictionaries
+                self._flatten_dict(value, flattened_dict, f"{new_key}-")
+            else:
+                # Add leaf values to the flattened dictionary
+                # Value must be cast to string here, so arg parser can cast from string to expected type later
+                flattened_dict[new_key] = str(value)
+
+
+def create_config_parser() -> configargparse.ArgumentParser:
+    parser = configargparse.ArgumentParser(
+        add_help=False,
+        config_file_parser_class=NestedYAMLConfigFileParser,
+        default_config_files=[
+            Path().home() / ".otava/conf.yaml",
+            Path().home() / ".otava/otava.yaml",
+        ],
+        allow_abbrev=False,  # required for correct parsing of nested values from config file
+    )
+    parser.add_argument('--config-file', is_config_file=True, help='Otava config file path', env_var="OTAVA_CONFIG")
+    GraphiteConfig.add_parser_args(parser.add_argument_group('Graphite Options', 'Options for Graphite configuration'))
+    GrafanaConfig.add_parser_args(parser.add_argument_group('Grafana Options', 'Options for Grafana configuration'))
+    SlackConfig.add_parser_args(parser.add_argument_group('Slack Options', 'Options for Slack configuration'))
+    PostgresConfig.add_parser_args(parser.add_argument_group('Postgres Options', 'Options for Postgres configuration'))
+    BigQueryConfig.add_parser_args(parser.add_argument_group('BigQuery Options', 'Options for BigQuery configuration'))
+    return parser
+
+
+def load_config_from_file(config_file: str, arg_overrides: Optional[List[str]] = None) -> Config:
+    if arg_overrides is None:
+        arg_overrides = []
+    arg_overrides.extend(["--config-file", config_file])
+    args, _ = create_config_parser().parse_known_args(args=arg_overrides)
+    return load_config_from_parser_args(args)

otava/grafana.py

@@ -30,6 +30,20 @@ class GrafanaConfig:
     user: str
     password: str
 
+    @staticmethod
+    def add_parser_args(arg_group):
+        arg_group.add_argument("--grafana-url", help="Grafana server URL", env_var="GRAFANA_ADDRESS")
+        arg_group.add_argument("--grafana-user", help="Grafana server user", env_var="GRAFANA_USER", default="admin")
+        arg_group.add_argument("--grafana-password", help="Grafana server password", env_var="GRAFANA_PASSWORD", default="admin")
+
+    @staticmethod
+    def from_parser_args(args):
+        return GrafanaConfig(
+            url=getattr(args, 'grafana_url', None),
+            user=getattr(args, 'grafana_user', None),
+            password=getattr(args, 'grafana_password', None)
+        )
+
 
 @dataclass
 class GrafanaError(Exception):

otava/graphite.py

@@ -31,6 +31,16 @@
 class GraphiteConfig:
     url: str
 
+    @staticmethod
+    def add_parser_args(arg_group):
+        arg_group.add_argument("--graphite-url", help="Graphite server URL", env_var="GRAPHITE_ADDRESS")
+
+    @staticmethod
+    def from_parser_args(args):
+        return GraphiteConfig(
+            url=getattr(args, 'graphite_url', None)
+        )
+
 
 @dataclass
 class DataPoint:

otava/main.py

@@ -15,21 +15,21 @@
 # specific language governing permissions and limitations
 # under the License.
 
-import argparse
 import copy
 import logging
 import sys
 from dataclasses import dataclass
 from datetime import datetime, timedelta
 from typing import Dict, List, Optional
 
+import configargparse as argparse
 import pytz
 from slack_sdk import WebClient
 
 from otava import config
 from otava.attributes import get_back_links
 from otava.bigquery import BigQuery, BigQueryError
-from otava.config import Config, ConfigError
+from otava.config import Config
 from otava.data_selector import DataSelector
 from otava.grafana import Annotation, Grafana, GrafanaError
 from otava.graphite import GraphiteError
@@ -514,19 +514,12 @@ def analysis_options_from_args(args: argparse.Namespace) -> AnalysisOptions:
     return conf
 
 
-def main():
-    try:
-        conf = config.load_config()
-    except ConfigError as err:
-        logging.error(err.message)
-        exit(1)
-    script_main(conf)
-
-
-def script_main(conf: Config, args: List[str] = None):
-    logging.basicConfig(format="%(levelname)s: %(message)s", level=logging.INFO)
-
-    parser = argparse.ArgumentParser(description="Hunts performance regressions in Fallout results")
+def create_otava_cli_parser() -> argparse.ArgumentParser:
+    parser = argparse.ArgumentParser(
+        description="Hunts performance regressions in Fallout results",
+        parents=[config.create_config_parser()],
+        allow_abbrev=False,  # required for correct parsing of nested values from config file
+    )
 
     subparsers = parser.add_subparsers(dest="command")
     list_tests_parser = subparsers.add_parser("list-tests", help="list available tests")
@@ -601,8 +594,17 @@ def script_main(conf: Config, args: List[str] = None):
         "validate", help="validates the tests and metrics defined in the configuration"
     )
 
+    return parser
+
+
+def script_main(conf: Config = None, args: List[str] = None):
+    logging.basicConfig(format="%(levelname)s: %(message)s", level=logging.INFO)
+    parser = create_otava_cli_parser()
+
     try:
-        args = parser.parse_args(args=args)
+        args, _ = parser.parse_known_args(args=args)
+        if conf is None:
+            conf = config.load_config_from_parser_args(args)
         otava = Otava(conf)
 
         if args.command == "list-groups":
@@ -727,5 +729,9 @@ def script_main(conf: Config, args: List[str] = None):
         exit(1)
 
 
+def main():
+    script_main()
+
+
 if __name__ == "__main__":
     main()

otava/postgres.py

@@ -33,6 +33,24 @@ class PostgresConfig:
     password: str
     database: str
 
+    @staticmethod
+    def add_parser_args(arg_group):
+        arg_group.add_argument("--postgres-hostname", help="PostgreSQL server hostname", env_var="POSTGRES_HOSTNAME")
+        arg_group.add_argument("--postgres-port", type=int, help="PostgreSQL server port", env_var="POSTGRES_PORT")
+        arg_group.add_argument("--postgres-username", help="PostgreSQL username", env_var="POSTGRES_USERNAME")
+        arg_group.add_argument("--postgres-password", help="PostgreSQL password", env_var="POSTGRES_PASSWORD")
+        arg_group.add_argument("--postgres-database", help="PostgreSQL database name", env_var="POSTGRES_DATABASE")
+
+    @staticmethod
+    def from_parser_args(args):
+        return PostgresConfig(
+            hostname=getattr(args, 'postgres_hostname', None),
+            port=getattr(args, 'postgres_port', None),
+            username=getattr(args, 'postgres_username', None),
+            password=getattr(args, 'postgres_password', None),
+            database=getattr(args, 'postgres_database', None)
+        )
+
 
 @dataclass
 class PostgresError(Exception):

otava/slack.py

@@ -36,6 +36,20 @@ class NotificationError(Exception):
 class SlackConfig:
     bot_token: str
 
+    @staticmethod
+    def add_parser_args(parser):
+        parser.add_argument(
+            "--slack-token",
+            help="Slack bot token to use for sending notifications",
+            env_var="SLACK_BOT_TOKEN",
+        )
+
+    @staticmethod
+    def from_parser_args(args):
+        return SlackConfig(
+            bot_token=getattr(args, "slack_token", None)
+        )
+
 
 class SlackNotification:
     tests_with_insufficient_data: List[str]