Address feedback from 0.7.0-rc3

.github/workflows/python-app.yml

@@ -22,9 +22,9 @@ name: Python application
 
 on:
   push:
-    branches: [master]
+    branches: [master, "0.7"]
   pull_request:
-    branches: [master]
+    branches: [master, "0.7"]
 
 jobs:
   build:

docs/CSV.md

@@ -20,7 +20,7 @@
 # Importing results from CSV
 
 > [!TIP]
-> See [otava.yaml](../examples/csv/otava.yaml) for the full example configuration.
+> See [otava.yaml](../examples/csv/config/otava.yaml) for the full example configuration.
 
 ## Tests
 

otava/bigquery.py

@@ -28,26 +28,10 @@
 
 @dataclass
 class BigQueryConfig:
-    NAME = "bigquery"
-
     project_id: str
     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,12 +14,12 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-import logging
+
+import os
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Dict, List, Optional
 
-import configargparse
 from expandvars import expandvars
 from ruamel.yaml import YAML
 
@@ -61,7 +61,7 @@ def load_tests(config: Dict, templates: Dict) -> Dict[str, TestConfig]:
         raise ConfigError("Property `tests` is not a dictionary")
 
     result = {}
-    for test_name, test_config in tests.items():
+    for (test_name, test_config) in tests.items():
         template_names = test_config.get("inherit", [])
         if not isinstance(template_names, List):
             template_names = [templates]
@@ -81,7 +81,7 @@ def load_test_groups(config: Dict, tests: Dict[str, TestConfig]) -> Dict[str, Li
         raise ConfigError("Property `test_groups` is not a dictionary")
 
     result = {}
-    for group_name, test_names in groups.items():
+    for (group_name, test_names) in groups.items():
         test_list = []
         if not isinstance(test_names, List):
             raise ConfigError(f"Test group {group_name} must be a list")
@@ -96,124 +96,107 @@ def load_test_groups(config: Dict, tests: Dict[str, TestConfig]) -> Dict[str, Li
     return result
 
 
-def expand_env_vars_recursive(obj):
-    """Recursively expand environment variables in all string values within a nested structure.
-
-    Raises ConfigError if any environment variables remain undefined after expansion.
-    """
-    if isinstance(obj, dict):
-        return {key: expand_env_vars_recursive(value) for key, value in obj.items()}
-    elif isinstance(obj, list):
-        return [expand_env_vars_recursive(item) for item in obj]
-    elif isinstance(obj, str):
-        return expandvars(obj, nounset=True)
-    else:
-        return obj
-
-
-def load_config_from_parser_args(args: configargparse.Namespace) -> Config:
-    config_file = getattr(args, "config_file", None)
-    if config_file is not None:
+def load_config_from(config_file: Path) -> Config:
+    """Loads config from the specified location"""
+    try:
+        content = expandvars(config_file.read_text(), nounset=True)
         yaml = YAML(typ="safe")
-        config = yaml.load(Path(config_file).read_text())
-
-        # Expand environment variables in the entire config after CLI argument replacement
-        try:
-            config = expand_env_vars_recursive(config)
-        except Exception as e:
-            raise ConfigError(f"Error expanding environment variables: {e}")
+        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"],
+            )
 
         templates = load_templates(config)
         tests = load_tests(config, templates)
         groups = load_test_groups(config, tests)
-    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.
-    """
-
-    CLI_CONFIG_SECTIONS = [
-        GraphiteConfig.NAME,
-        GrafanaConfig.NAME,
-        SlackConfig.NAME,
-        PostgresConfig.NAME,
-        BigQueryConfig.NAME,
+
+        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",
     ]
 
-    def parse(self, stream):
-        yaml = YAML(typ="safe")
-        config_data = yaml.load(stream)
-        if config_data is None:
-            return {}
-
-        flattened_dict = {}
-        for key, value in config_data.items():
-            if key in self.CLI_CONFIG_SECTIONS:
-                # Flatten only the config sections that correspond to CLI arguments
-                self._flatten_dict(value, flattened_dict, f"{key}-")
-            # Ignore other sections like 'templates' and 'tests' - they shouldn't become CLI arguments
-        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)
+    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}")

otava/grafana.py

@@ -26,26 +26,10 @@
 
 @dataclass
 class GrafanaConfig:
-    NAME = "grafana"
-
     url: str
     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

@@ -29,20 +29,8 @@
 
 @dataclass
 class GraphiteConfig:
-    NAME = "graphite"
-
     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: