This is an automated email from the ASF dual-hosted git repository. asorokoumov pushed a commit to branch fixup-0.7.0-rc3 in repository https://gitbox.apache.org/repos/asf/otava.git
commit fcfc462c59d1d19af4d15a8fb498ea80117c36e2 Author: Alex Sorokoumov <[email protected]> AuthorDate: Thu Nov 27 20:33:54 2025 -0800 Revert "Fix config parsing (#91)" This reverts commit 484aaef8493a076b42d1b0e92679d9edb87fb043. --- otava/bigquery.py | 2 -- otava/config.py | 15 +--------- otava/grafana.py | 2 -- otava/graphite.py | 2 -- otava/main.py | 1 - otava/postgres.py | 2 -- otava/slack.py | 2 -- tests/config_test.py | 78 +--------------------------------------------------- 8 files changed, 2 insertions(+), 102 deletions(-) diff --git a/otava/bigquery.py b/otava/bigquery.py index cf5fafd..c9b77a6 100644 --- a/otava/bigquery.py +++ b/otava/bigquery.py @@ -28,8 +28,6 @@ from otava.test_config import BigQueryTestConfig @dataclass class BigQueryConfig: - NAME = "bigquery" - project_id: str dataset: str credentials: str diff --git a/otava/config.py b/otava/config.py index 345dede..c0e8d54 100644 --- a/otava/config.py +++ b/otava/config.py @@ -127,26 +127,13 @@ class NestedYAMLConfigFileParser(configargparse.ConfigFileParser): 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, - ] - 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 + self._flatten_dict(config_data, flattened_dict) return flattened_dict def _flatten_dict(self, nested_dict, flattened_dict, prefix=''): diff --git a/otava/grafana.py b/otava/grafana.py index b853f3a..bc90998 100644 --- a/otava/grafana.py +++ b/otava/grafana.py @@ -26,8 +26,6 @@ from requests.exceptions import HTTPError @dataclass class GrafanaConfig: - NAME = "grafana" - url: str user: str password: str diff --git a/otava/graphite.py b/otava/graphite.py index 69a7592..7c3709d 100644 --- a/otava/graphite.py +++ b/otava/graphite.py @@ -29,8 +29,6 @@ from otava.util import parse_datetime @dataclass class GraphiteConfig: - NAME = "graphite" - url: str @staticmethod diff --git a/otava/main.py b/otava/main.py index 1c2e1e4..60d6e37 100644 --- a/otava/main.py +++ b/otava/main.py @@ -518,7 +518,6 @@ def create_otava_cli_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( description="Hunts performance regressions in Fallout results", parents=[config.create_config_parser()], - config_file_parser_class=config.NestedYAMLConfigFileParser, allow_abbrev=False, # required for correct parsing of nested values from config file ) diff --git a/otava/postgres.py b/otava/postgres.py index 7a2aaa0..d014bb6 100644 --- a/otava/postgres.py +++ b/otava/postgres.py @@ -27,8 +27,6 @@ from otava.test_config import PostgresTestConfig @dataclass class PostgresConfig: - NAME = "postgres" - hostname: str port: int username: str diff --git a/otava/slack.py b/otava/slack.py index a0d4bc3..0d9b751 100644 --- a/otava/slack.py +++ b/otava/slack.py @@ -34,8 +34,6 @@ class NotificationError(Exception): @dataclass class SlackConfig: - NAME = "slack" - bot_token: str @staticmethod diff --git a/tests/config_test.py b/tests/config_test.py index 5ce500e..f1e4a1b 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -15,11 +15,10 @@ # specific language governing permissions and limitations # under the License. import os -from io import StringIO import pytest -from otava.config import NestedYAMLConfigFileParser, load_config_from_file +from otava.config import load_config_from_file from otava.test_config import CsvTestConfig, GraphiteTestConfig, HistoStatTestConfig @@ -134,78 +133,3 @@ def test_configuration_substitutions(config_property): assert accessor(config) == cli_config_value finally: os.environ.pop(config_property[2]) - - -def test_config_section_yaml_parser_flattens_only_config_sections(): - """Test that NestedYAMLConfigFileParser only flattens the specified config sections.""" - - parser = NestedYAMLConfigFileParser() - test_yaml = """ -graphite: - url: http://example.com - timeout: 30 -slack: - bot_token: test-token - channel: "#alerts" -postgres: - hostname: localhost - port: 5432 -templates: - aggregate_mem: - type: postgres - time_column: commit_ts - attributes: [experiment_id, config_id, commit] - metrics: - process_cumulative_rate_mean: - direction: 1 - scale: 1 - process_cumulative_rate_stderr: - direction: -1 - scale: 1 - process_cumulative_rate_diff: - direction: -1 - scale: 1 - query: | - SELECT e.commit, - e.commit_ts, - r.process_cumulative_rate_mean, - r.process_cumulative_rate_stderr, - r.process_cumulative_rate_diff, - r.experiment_id, - r.config_id - FROM results r - INNER JOIN configs c ON r.config_id = c.id - INNER JOIN experiments e ON r.experiment_id = e.id - WHERE e.exclude_from_analysis = false AND - e.branch = 'trunk' AND - e.username = 'ci' AND - c.store = 'MEM' AND - c.cache = true AND - c.benchmark = 'aggregate' AND - c.instance_type = 'ec2i3.large' - ORDER BY e.commit_ts ASC; -""" - - stream = StringIO(test_yaml) - result = parser.parse(stream) - - # Should flatten config sections - expected_keys = { - 'graphite-url', 'graphite-timeout', - 'slack-bot-token', 'slack-channel', - 'postgres-hostname', 'postgres-port' - } - - assert set(result.keys()) == expected_keys - assert result['graphite-url'] == 'http://example.com' - assert result['graphite-timeout'] == '30' - assert result['slack-bot-token'] == 'test-token' - assert result['slack-channel'] == '#alerts' - assert result['postgres-hostname'] == 'localhost' - assert result['postgres-port'] == '5432' - - # Should NOT contain any keys from ignored sections - ignored_sections = {'templates', 'tests', 'test_groups'} - for key in result.keys(): - section = key.split('-')[0] - assert section not in ignored_sections, f"Found key '{key}' from ignored section '{section}'"
