Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions otava/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

@dataclass
class BigQueryConfig:
NAME = "bigquery"

project_id: str
dataset: str
credentials: str
Expand Down
15 changes: 14 additions & 1 deletion otava/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,26 @@ 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 = {}
self._flatten_dict(config_data, 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=''):
Expand Down
2 changes: 2 additions & 0 deletions otava/grafana.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

@dataclass
class GrafanaConfig:
NAME = "grafana"

url: str
user: str
password: str
Expand Down
2 changes: 2 additions & 0 deletions otava/graphite.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

@dataclass
class GraphiteConfig:
NAME = "graphite"

url: str

@staticmethod
Expand Down
1 change: 1 addition & 0 deletions otava/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ 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
)

Expand Down
2 changes: 2 additions & 0 deletions otava/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

@dataclass
class PostgresConfig:
NAME = "postgres"

hostname: str
port: int
username: str
Expand Down
2 changes: 2 additions & 0 deletions otava/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class NotificationError(Exception):

@dataclass
class SlackConfig:
NAME = "slack"

bot_token: str

@staticmethod
Expand Down
78 changes: 77 additions & 1 deletion tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
# specific language governing permissions and limitations
# under the License.
import os
from io import StringIO

import pytest

from otava.config import load_config_from_file
from otava.config import NestedYAMLConfigFileParser, load_config_from_file
from otava.test_config import CsvTestConfig, GraphiteTestConfig, HistoStatTestConfig


Expand Down Expand Up @@ -133,3 +134,78 @@ 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}'"