From bc0ee05b0f8dccd3b10bd3e6c35b318c4545425c Mon Sep 17 00:00:00 2001 From: MrlixiangWE <102979255+MrlixiangWE@users.noreply.github.com> Date: Tue, 4 Aug 2026 10:30:24 +0800 Subject: [PATCH] Make CSV importer configurable via ConfigArgParse --- docs/CSV.md | 18 ++++- examples/csv/config/otava.yaml | 2 +- otava/config.py | 16 +++- otava/csv_options.py | 33 ++++++++ otava/test_config.py | 17 +++- tests/cli_help_test.py | 77 +++++++++++++++++-- tests/config_test.py | 29 +++++++ tests/resources/sample_config.yaml | 3 + tests/resources/substitution_test_config.yaml | 4 + 9 files changed, 182 insertions(+), 17 deletions(-) diff --git a/docs/CSV.md b/docs/CSV.md index 6d674bf8..eef80132 100644 --- a/docs/CSV.md +++ b/docs/CSV.md @@ -34,9 +34,25 @@ tests: metrics: [metric1, metric2] csv_options: delimiter: ',' - quotechar: "'" + quote_char: "'" ``` +## CSV options + +The delimiter and quote character can also be configured globally: + +```yaml +csv: + delimiter: ';' + quote_char: "'" +``` + +The corresponding command-line options are `--csv-delimiter` and +`--csv-quote-char`. They can also be set through the `CSV_DELIMITER` and +`CSV_QUOTE_CHAR` environment variables. Explicit global values override the +`csv_options` values of individual tests; without a global value, existing +per-test settings and defaults are unchanged. + ## Example ```bash diff --git a/examples/csv/config/otava.yaml b/examples/csv/config/otava.yaml index d91f4ea7..7989aa19 100644 --- a/examples/csv/config/otava.yaml +++ b/examples/csv/config/otava.yaml @@ -24,4 +24,4 @@ tests: metrics: [metric1, metric2] csv_options: delimiter: "," - quotechar: "'" + quote_char: "'" diff --git a/otava/config.py b/otava/config.py index d9cd9866..f89a2b79 100644 --- a/otava/config.py +++ b/otava/config.py @@ -23,6 +23,7 @@ from ruamel.yaml import YAML from otava.bigquery import BigQueryConfig +from otava.csv_options import CsvConfig from otava.grafana import GrafanaConfig from otava.graphite import GraphiteConfig from otava.postgres import PostgresConfig @@ -33,6 +34,7 @@ @dataclass class Config: + csv: CsvConfig graphite: Optional[GraphiteConfig] grafana: Optional[GrafanaConfig] tests: Dict[str, TestConfig] @@ -54,7 +56,9 @@ def load_templates(config: Dict) -> Dict[str, Dict]: return templates -def load_tests(config: Dict, templates: Dict) -> Dict[str, TestConfig]: +def load_tests( + config: Dict, templates: Dict, csv_config: Optional[CsvConfig] = None +) -> Dict[str, TestConfig]: tests = config.get("tests", {}) if not isinstance(tests, Dict): raise ConfigError("Property `tests` is not a dictionary") @@ -69,7 +73,7 @@ def load_tests(config: Dict, templates: Dict) -> Dict[str, TestConfig]: except KeyError as e: raise ConfigError(f"Template {e.args[0]} referenced in test {test_name} not found") test_config = merge_dict_list(template_list + [test_config]) - result[test_name] = create_test_config(test_name, test_config) + result[test_name] = create_test_config(test_name, test_config, csv_config) return result @@ -96,13 +100,14 @@ def load_test_groups(config: Dict, tests: Dict[str, TestConfig]) -> Dict[str, Li def load_config_from_parser_args(args: configargparse.Namespace) -> Config: + csv_config = CsvConfig.from_parser_args(args) config_file = getattr(args, "config_file", None) if config_file is not None: yaml = YAML(typ="safe") config = yaml.load(Path(config_file).read_text()) templates = load_templates(config) - tests = load_tests(config, templates) + tests = load_tests(config, templates, csv_config) groups = load_test_groups(config, tests) else: logging.warning("Otava configuration file not found or not specified") @@ -110,6 +115,7 @@ def load_config_from_parser_args(args: configargparse.Namespace) -> Config: groups = {} return Config( + csv=csv_config, graphite=GraphiteConfig.from_parser_args(args), grafana=GrafanaConfig.from_parser_args(args), slack=SlackConfig.from_parser_args(args), @@ -128,6 +134,7 @@ class NestedYAMLConfigFileParser(configargparse.ConfigFileParser): """ CLI_CONFIG_SECTIONS = [ + CsvConfig.NAME, GraphiteConfig.NAME, GrafanaConfig.NAME, SlackConfig.NAME, @@ -174,7 +181,8 @@ def get_syntax_description(self): def add_service_option_groups(parser) -> None: - """Add Graphite, Grafana, Slack, Postgres, and BigQuery option groups to a parser.""" + """Add importer and integration option groups to a parser.""" + CsvConfig.add_parser_args(parser.add_argument_group('CSV Options', 'Options for CSV configuration')) 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')) diff --git a/otava/csv_options.py b/otava/csv_options.py index 2c9ddb21..0564c2a2 100644 --- a/otava/csv_options.py +++ b/otava/csv_options.py @@ -17,6 +17,39 @@ import enum from dataclasses import dataclass +from typing import Optional + +import configargparse + + +@dataclass +class CsvConfig: + NAME = "csv" + + delimiter: Optional[str] = None + quote_char: Optional[str] = None + + @staticmethod + def add_parser_args(arg_group): + arg_group.add_argument( + "--csv-delimiter", + help="CSV delimiter", + env_var="CSV_DELIMITER", + default=configargparse.SUPPRESS, + ) + arg_group.add_argument( + "--csv-quote-char", + help="CSV quote character", + env_var="CSV_QUOTE_CHAR", + default=configargparse.SUPPRESS, + ) + + @staticmethod + def from_parser_args(args): + return CsvConfig( + delimiter=getattr(args, "csv_delimiter", None), + quote_char=getattr(args, "csv_quote_char", None), + ) @dataclass diff --git a/otava/test_config.py b/otava/test_config.py index 5cc57c95..140d8148 100644 --- a/otava/test_config.py +++ b/otava/test_config.py @@ -19,7 +19,7 @@ from dataclasses import dataclass from typing import Dict, List, Optional -from otava.csv_options import CsvOptions +from otava.csv_options import CsvConfig, CsvOptions @dataclass @@ -199,7 +199,9 @@ def fully_qualified_metric_names(self) -> List[str]: return list(self.metrics.keys()) -def create_test_config(name: str, config: Dict) -> TestConfig: +def create_test_config( + name: str, config: Dict, csv_config: Optional[CsvConfig] = None +) -> TestConfig: """ Loads properties of a test from a dictionary read from otava's config file This dictionary must have the `type` property to determine the type of the test. @@ -208,7 +210,7 @@ def create_test_config(name: str, config: Dict) -> TestConfig: """ test_type = config.get("type") if test_type == "csv": - return create_csv_test_config(name, config) + return create_csv_test_config(name, config, csv_config) elif test_type == "graphite": return create_graphite_test_config(name, config) elif test_type == "histostat": @@ -225,7 +227,9 @@ def create_test_config(name: str, config: Dict) -> TestConfig: raise TestConfigError(f"Unknown test type {test_type} for test {name}") -def create_csv_test_config(test_name: str, test_info: Dict) -> CsvTestConfig: +def create_csv_test_config( + test_name: str, test_info: Dict, csv_config: Optional[CsvConfig] = None +) -> CsvTestConfig: csv_options = CsvOptions() try: file = test_info["file"] @@ -257,6 +261,11 @@ def create_csv_test_config(test_name: str, test_info: Dict) -> CsvTestConfig: if test_info.get("csv_options"): csv_options.delimiter = test_info["csv_options"].get("delimiter", ",") csv_options.quote_char = test_info["csv_options"].get("quote_char", '"') + if csv_config is not None: + if csv_config.delimiter is not None: + csv_options.delimiter = csv_config.delimiter + if csv_config.quote_char is not None: + csv_options.quote_char = csv_config.quote_char return CsvTestConfig( test_name, file, diff --git a/tests/cli_help_test.py b/tests/cli_help_test.py index 2397b553..32946679 100644 --- a/tests/cli_help_test.py +++ b/tests/cli_help_test.py @@ -49,7 +49,8 @@ def test_otava_help_output(): assert ( result.stdout == """\ -usage: otava [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -73,6 +74,14 @@ def test_otava_help_output(): --config-file CONFIG_FILE Otava config file path [env var: OTAVA_CONFIG] +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -147,7 +156,8 @@ def test_otava_analyze_help_output(): magnitude_option = " -M MAGNITUDE, --magnitude MAGNITUDE" usage_and_options = f"""\ -usage: otava analyze [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava analyze [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -218,6 +228,14 @@ def test_otava_analyze_help_output(): --orig-edivisive use the original edivisive algorithm with no windowing and weak change points analysis improvements +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -278,7 +296,8 @@ def test_otava_list_tests_help_output(): assert ( result.stdout == """\ -usage: otava list-tests [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava list-tests [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -298,6 +317,14 @@ def test_otava_list_tests_help_output(): --config-file CONFIG_FILE Otava config file path [env var: OTAVA_CONFIG] +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -358,7 +385,8 @@ def test_otava_list_metrics_help_output(): assert ( result.stdout == """\ -usage: otava list-metrics [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava list-metrics [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -378,6 +406,14 @@ def test_otava_list_metrics_help_output(): --config-file CONFIG_FILE Otava config file path [env var: OTAVA_CONFIG] +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -439,7 +475,8 @@ def test_otava_list_groups_help_output(): assert ( result.stdout == """\ -usage: otava list-groups [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava list-groups [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -455,6 +492,14 @@ def test_otava_list_groups_help_output(): --config-file CONFIG_FILE Otava config file path [env var: OTAVA_CONFIG] +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -515,7 +560,8 @@ def test_otava_remove_annotations_help_output(): assert ( result.stdout == """\ -usage: otava remove-annotations [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava remove-annotations [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] @@ -537,6 +583,14 @@ def test_otava_remove_annotations_help_output(): Otava config file path [env var: OTAVA_CONFIG] --force don't ask questions, just do it +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration @@ -597,7 +651,8 @@ def test_otava_validate_help_output(): assert ( result.stdout == """\ -usage: otava validate [-h] [--config-file CONFIG_FILE] [--graphite-url GRAPHITE_URL] +usage: otava validate [-h] [--config-file CONFIG_FILE] [--csv-delimiter CSV_DELIMITER] + [--csv-quote-char CSV_QUOTE_CHAR] [--graphite-url GRAPHITE_URL] [--grafana-url GRAFANA_URL] [--grafana-user GRAFANA_USER] [--grafana-password GRAFANA_PASSWORD] [--slack-token SLACK_TOKEN] [--postgres-hostname POSTGRES_HOSTNAME] [--postgres-port POSTGRES_PORT] @@ -613,6 +668,14 @@ def test_otava_validate_help_output(): --config-file CONFIG_FILE Otava config file path [env var: OTAVA_CONFIG] +CSV Options: + Options for CSV configuration + + --csv-delimiter CSV_DELIMITER + CSV delimiter [env var: CSV_DELIMITER] + --csv-quote-char CSV_QUOTE_CHAR + CSV quote character [env var: CSV_QUOTE_CHAR] + Graphite Options: Options for Graphite configuration diff --git a/tests/config_test.py b/tests/config_test.py index 58b223f2..596d156c 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -54,6 +54,8 @@ def test_load_csv_tests(): assert len(test.metrics) == 2 assert len(test.attributes) == 1 assert test.file == "tests/resources/sample.csv" + assert test.csv_options.delimiter == "," + assert test.csv_options.quote_char == '"' test = tests["local2"] assert isinstance(test, CsvTestConfig) @@ -100,6 +102,8 @@ def test_load_histostat_config(): ("postgres_username", lambda c: c.postgres.username, "POSTGRES_USERNAME", "--postgres-username"), ("postgres_password", lambda c: c.postgres.password, "POSTGRES_PASSWORD", "--postgres-password"), ("postgres_database", lambda c: c.postgres.database, "POSTGRES_DATABASE", "--postgres-database"), + ("csv_delimiter", lambda c: c.csv.delimiter, "CSV_DELIMITER", "--csv-delimiter", ";", "|", "\t"), + ("csv_quote_char", lambda c: c.csv.quote_char, "CSV_QUOTE_CHAR", "--csv-quote-char", "'", "|", "`"), ], ids=lambda v: v[0], # use the property name for the parameterized test name ) @@ -216,6 +220,31 @@ def test_config_section_yaml_parser_flattens_only_config_sections(): assert section not in ignored_sections, f"Found key '{key}' from ignored section '{section}'" +def test_csv_configargparse_options_apply_to_csv_tests(): + config = load_config_from_file( + "tests/resources/sample_config.yaml", + arg_overrides=["--csv-delimiter", ";", "--csv-quote-char", "'"], + ) + + assert config.tests["local1"].csv_options.delimiter == ";" + assert config.tests["local1"].csv_options.quote_char == "'" + assert config.tests["local2"].csv_options.delimiter == ";" + assert config.tests["local2"].csv_options.quote_char == "'" + + +@pytest.mark.parametrize( + "args", + [ + ["--csv-delimiter", ";", "analyze", "local1"], + ["analyze", "local1", "--csv-delimiter", ";"], + ], +) +def test_csv_cli_options_can_appear_before_or_after_subcommand(args): + parsed = create_otava_cli_parser().parse_args(args) + + assert parsed.csv_delimiter == ";" + + def test_cli_precedence_over_env_vars(): """Test that CLI arguments take precedence over environment variables.""" diff --git a/tests/resources/sample_config.yaml b/tests/resources/sample_config.yaml index 995204aa..907f1185 100644 --- a/tests/resources/sample_config.yaml +++ b/tests/resources/sample_config.yaml @@ -89,6 +89,9 @@ tests: time_column: time metrics: [metric1, metric2] attributes: [commit] + csv_options: + delimiter: "," + quote_char: '"' local2: type: csv diff --git a/tests/resources/substitution_test_config.yaml b/tests/resources/substitution_test_config.yaml index 6b6b0245..a1d767ad 100644 --- a/tests/resources/substitution_test_config.yaml +++ b/tests/resources/substitution_test_config.yaml @@ -37,3 +37,7 @@ postgres: username: config_postgres_username password: config_postgres_password database: config_postgres_database + +csv: + delimiter: ";" + quote_char: "'"