From 573e54fee7b03894f29a64dba272d122c730be10 Mon Sep 17 00:00:00 2001 From: MrlixiangWE <102979255+MrlixiangWE@users.noreply.github.com> Date: Sun, 23 Aug 2026 07:28:30 +0000 Subject: [PATCH] Make CSV importer configurable via ConfigArgParse CSV delimiter and quote character could only be set per test in YAML. They can now also be set through a top-level csv section in the config file, the --csv-delimiter and --csv-quote-char options, and the CSV_DELIMITER and CSV_QUOTE_CHAR environment variables. Explicit global values override per-test csv_options; otherwise existing behavior is unchanged. The per-test YAML key is renamed from quotechar to quote_char. The loader only ever read quote_char, so the documented quotechar spelling was silently ignored and no working configuration depends on it. Closes #119 --- docs/CSV.md | 18 ++++- examples/csv/config/otava.yaml | 2 +- otava/config.py | 16 +++- otava/csv_options.py | 42 ++++++++++ otava/test_config.py | 22 ++++-- tests/cli_help_test.py | 77 +++++++++++++++++-- tests/cli_options_test.py | 2 +- tests/config_test.py | 70 ++++++++++++++++- tests/csv_e2e_test.py | 2 +- tests/resources/sample_config.yaml | 3 + tests/resources/substitution_test_config.yaml | 4 + 11 files changed, 236 insertions(+), 22 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 ae48ad42..7fecea5c 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.influxdb import InfluxDBConfig @@ -34,6 +35,7 @@ @dataclass class Config: + csv: CsvConfig graphite: Optional[GraphiteConfig] grafana: Optional[GrafanaConfig] tests: Dict[str, TestConfig] @@ -56,7 +58,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") @@ -71,7 +75,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 @@ -98,13 +102,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") @@ -112,6 +117,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), @@ -131,6 +137,7 @@ class NestedYAMLConfigFileParser(configargparse.ConfigFileParser): """ CLI_CONFIG_SECTIONS = [ + CsvConfig.NAME, GraphiteConfig.NAME, GrafanaConfig.NAME, SlackConfig.NAME, @@ -178,7 +185,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..7ba82c5b 100644 --- a/otava/csv_options.py +++ b/otava/csv_options.py @@ -17,6 +17,48 @@ import enum from dataclasses import dataclass +from typing import Optional + +import configargparse + + +def single_character(value: str) -> str: + """The csv module only accepts single-character delimiters and quote characters.""" + if len(value) != 1: + raise configargparse.ArgumentTypeError(f"must be a single character, got {value!r}") + return value + + +@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", + type=single_character, + default=configargparse.SUPPRESS, + ) + arg_group.add_argument( + "--csv-quote-char", + help="CSV quote character", + env_var="CSV_QUOTE_CHAR", + type=single_character, + 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 db24d422..75c2025d 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 @@ -235,7 +235,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. @@ -244,7 +246,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": @@ -263,7 +265,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"] @@ -293,8 +297,14 @@ def create_csv_test_config(test_name: str, test_info: Dict) -> CsvTestConfig: raise TestConfigError(f"Attributes of the test {test_name} must be a list") 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", '"') + per_test_options = test_info["csv_options"] + csv_options.delimiter = per_test_options.get("delimiter", ",") + csv_options.quote_char = per_test_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 f2ce443d..06cb974a 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] @@ -75,6 +76,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 @@ -159,7 +168,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] @@ -228,6 +238,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 @@ -301,7 +319,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] @@ -323,6 +342,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 @@ -393,7 +420,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] @@ -415,6 +443,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 @@ -486,7 +522,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] @@ -504,6 +541,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 @@ -574,7 +619,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] @@ -599,6 +645,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 @@ -669,7 +723,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] @@ -687,6 +742,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/cli_options_test.py b/tests/cli_options_test.py index d578f14b..d5f4aebe 100644 --- a/tests/cli_options_test.py +++ b/tests/cli_options_test.py @@ -115,7 +115,7 @@ def _create_csv_config_file_for_test(td_path: Path): metrics: [metric1, metric2] csv_options: delimiter: "," - quotechar: "'" + quote_char: "'" """ ) config_path = td_path / "otava.yaml" diff --git a/tests/config_test.py b/tests/config_test.py index 58b223f2..d0a031d8 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -25,7 +25,12 @@ load_config_from_file, ) from otava.main import create_otava_cli_parser -from otava.test_config import CsvTestConfig, GraphiteTestConfig, HistoStatTestConfig +from otava.test_config import ( + CsvTestConfig, + GraphiteTestConfig, + HistoStatTestConfig, + create_csv_test_config, +) def test_load_graphite_tests(): @@ -54,6 +59,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) @@ -66,6 +73,40 @@ def test_load_csv_tests(): assert test.file == "tests/resources/sample.csv" +def test_per_test_csv_quote_character_config_name(): + test = create_csv_test_config( + "local", + {"file": "sample.csv", "metrics": [], "csv_options": {"quote_char": "|"}}, + ) + + assert test.csv_options.quote_char == "|" + + +def test_global_csv_quote_character_config_name(): + parser = NestedYAMLConfigFileParser() + result = parser.parse(StringIO("csv:\n quote_char: '|'\n")) + + assert result == {"csv-quote-char": "|"} + + +@pytest.mark.parametrize("option", ["--csv-delimiter", "--csv-quote-char"]) +def test_csv_options_reject_multiple_characters(option, tmp_path): + config_file = tmp_path / "otava.yaml" + config_file.write_text("tests: {}\n") + + with pytest.raises(SystemExit): + load_config_from_file(str(config_file), arg_overrides=[option, "::"]) + + +@pytest.mark.parametrize("key", ["delimiter", "quote_char"]) +def test_csv_config_file_options_reject_multiple_characters(key, tmp_path): + config_file = tmp_path / "otava.yaml" + config_file.write_text(f'csv:\n {key}: "::"\ntests: {{}}\n') + + with pytest.raises(SystemExit): + load_config_from_file(str(config_file)) + + def test_load_test_groups(): config = load_config_from_file("tests/resources/sample_config.yaml") groups = config.test_groups @@ -100,6 +141,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 +259,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/csv_e2e_test.py b/tests/csv_e2e_test.py index 6afb4226..25c90a33 100644 --- a/tests/csv_e2e_test.py +++ b/tests/csv_e2e_test.py @@ -63,7 +63,7 @@ def test_analyze_csv(): metrics: [metric1, metric2] csv_options: delimiter: "," - quotechar: "'" + quote_char: "'" """ ) expected_output = textwrap.dedent( diff --git a/tests/resources/sample_config.yaml b/tests/resources/sample_config.yaml index 995204aa..ca442e0b 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: "'"