This is an automated email from the ASF dual-hosted git repository.
asorokoumov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/otava.git
The following commit(s) were added to refs/heads/master by this push:
new 5276f24 Fix multiple issues discovered via running examples (#95)
5276f24 is described below
commit 5276f24de6f0853f8304374dd1b3772501863c3c
Author: Alex Sorokoumov <[email protected]>
AuthorDate: Wed Nov 12 23:18:37 2025 -0800
Fix multiple issues discovered via running examples (#95)
* Fix docker run commands in examples
* Bump year in the postgres example data
By default, Otava looks 1 year back. Example data is too old.
* Expand all env variables and fix PostgreSQL example
This commit fixes behavior removed in #86 - expand all env variables,
not just ones matching CLI arguments. There are users relying on this
behavior so we should not break it without longer discussion and a
major release.
---
docs/GRAFANA.md | 2 +-
docs/GRAPHITE.md | 2 +-
docs/POSTGRESQL.md | 2 +-
examples/postgresql/init-db/schema.sql | 12 +-
otava/config.py | 26 +++-
pyproject.toml | 1 +
tests/config_test.py | 232 ++++++++++++++++++++++++++++++++-
uv.lock | 13 +-
8 files changed, 277 insertions(+), 13 deletions(-)
diff --git a/docs/GRAFANA.md b/docs/GRAFANA.md
index 13e8210..922c0ad 100644
--- a/docs/GRAFANA.md
+++ b/docs/GRAFANA.md
@@ -61,7 +61,7 @@ docker-compose -f examples/graphite/docker-compose.yaml up
--force-recreate --al
Run otava in another tab:
```bash
-docker-compose -f examples/graphite/docker-compose.yaml run otava otava
analyze my-product.test --since=-10m --update-grafana
+docker-compose -f examples/graphite/docker-compose.yaml run otava analyze
my-product.test --since=-10m --update-grafana
```
Expected output:
diff --git a/docs/GRAPHITE.md b/docs/GRAPHITE.md
index d7987b9..54d0e83 100644
--- a/docs/GRAPHITE.md
+++ b/docs/GRAPHITE.md
@@ -105,7 +105,7 @@ docker-compose -f examples/graphite/docker-compose.yaml up
--force-recreate --al
Run otava in another tab:
```bash
-docker-compose -f examples/graphite/docker-compose.yaml run otava otava
analyze my-product.test --since=-10m
+docker-compose -f examples/graphite/docker-compose.yaml run otava analyze
my-product.test --since=-10m
```
Expected output:
diff --git a/docs/POSTGRESQL.md b/docs/POSTGRESQL.md
index 115483a..7ecda0b 100644
--- a/docs/POSTGRESQL.md
+++ b/docs/POSTGRESQL.md
@@ -99,7 +99,7 @@ docker-compose -f examples/postgresql/docker-compose.yaml up
--force-recreate --
Run Otava in the other tab to show results for a single test `aggregate_mem`
and update the database with newly found change points:
```bash
-docker-compose -f examples/postgresql/docker-compose.yaml run --build otava
bin/otava analyze aggregate_mem --update-postgres
+docker-compose -f examples/postgresql/docker-compose.yaml run --build otava
analyze aggregate_mem --update-postgres
```
Expected output:
diff --git a/examples/postgresql/init-db/schema.sql
b/examples/postgresql/init-db/schema.sql
index 632cf3f..1092331 100644
--- a/examples/postgresql/init-db/schema.sql
+++ b/examples/postgresql/init-db/schema.sql
@@ -75,12 +75,12 @@ INSERT INTO configs (id, benchmark, store, instance_type,
cache) VALUES
INSERT INTO experiments
(id, ts, branch, commit, commit_ts, username, details_url)
VALUES
- ('aggregate-36e5ccd2', '2024-03-14 12:03:02+00', 'trunk', '36e5ccd2',
'2024-03-13 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-36e5ccd2'),
- ('aggregate-d5460f38', '2024-03-27 12:03:02+00', 'trunk', 'd5460f38',
'2024-03-25 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-d5460f38'),
- ('aggregate-bc9425cb', '2024-04-01 12:03:02+00', 'trunk', 'bc9425cb',
'2024-04-02 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-bc9425cb'),
- ('aggregate-14df1b11', '2024-04-07 12:03:02+00', 'trunk', '14df1b11',
'2024-04-06 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-14df1b11'),
- ('aggregate-ac40c0d8', '2024-04-14 12:03:02+00', 'trunk', 'ac40c0d8',
'2024-04-13 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-ac40c0d8'),
- ('aggregate-0af4ccbc', '2024-04-28 12:03:02+00', 'trunk', '0af4ccbc',
'2024-04-27 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-0af4ccbc');
+ ('aggregate-36e5ccd2', '2025-03-14 12:03:02+00', 'trunk', '36e5ccd2',
'2025-03-13 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-36e5ccd2'),
+ ('aggregate-d5460f38', '2025-03-27 12:03:02+00', 'trunk', 'd5460f38',
'2025-03-25 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-d5460f38'),
+ ('aggregate-bc9425cb', '2025-04-01 12:03:02+00', 'trunk', 'bc9425cb',
'2025-04-02 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-bc9425cb'),
+ ('aggregate-14df1b11', '2025-04-07 12:03:02+00', 'trunk', '14df1b11',
'2025-04-06 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-14df1b11'),
+ ('aggregate-ac40c0d8', '2025-04-14 12:03:02+00', 'trunk', 'ac40c0d8',
'2025-04-13 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-ac40c0d8'),
+ ('aggregate-0af4ccbc', '2025-04-28 12:03:02+00', 'trunk', '0af4ccbc',
'2025-04-27 10:03:02+00', 'ci',
'https://example.com/experiments/aggregate-0af4ccbc');
INSERT INTO results (experiment_id, config_id, process_cumulative_rate_mean,
process_cumulative_rate_stderr, process_cumulative_rate_diff)
diff --git a/otava/config.py b/otava/config.py
index 345dede..ca1d587 100644
--- a/otava/config.py
+++ b/otava/config.py
@@ -20,6 +20,7 @@ from pathlib import Path
from typing import Dict, List, Optional
import configargparse
+from expandvars import expandvars
from ruamel.yaml import YAML
from otava.bigquery import BigQueryConfig
@@ -60,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]
@@ -80,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")
@@ -95,12 +96,33 @@ 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:
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}")
+
templates = load_templates(config)
tests = load_tests(config, templates)
groups = load_test_groups(config, tests)
diff --git a/pyproject.toml b/pyproject.toml
index 24e3b36..6bb1409 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -48,6 +48,7 @@ dependencies = [
"google-cloud-bigquery>=3.25.0",
"pg8000>=1.31.2",
"configargparse>=1.7.1",
+ "expandvars>=0.12.0",
]
[project.optional-dependencies]
diff --git a/tests/config_test.py b/tests/config_test.py
index 5ce500e..4f66b95 100644
--- a/tests/config_test.py
+++ b/tests/config_test.py
@@ -15,11 +15,19 @@
# specific language governing permissions and limitations
# under the License.
import os
+import tempfile
from io import StringIO
import pytest
+from expandvars import UnboundVariable
-from otava.config import NestedYAMLConfigFileParser, load_config_from_file
+from otava.config import (
+ NestedYAMLConfigFileParser,
+ create_config_parser,
+ expand_env_vars_recursive,
+ load_config_from_file,
+ load_config_from_parser_args,
+)
from otava.test_config import CsvTestConfig, GraphiteTestConfig,
HistoStatTestConfig
@@ -209,3 +217,225 @@ templates:
for key in result.keys():
section = key.split('-')[0]
assert section not in ignored_sections, f"Found key '{key}' from
ignored section '{section}'"
+
+
+def test_expand_env_vars_recursive():
+ """Test the expand_env_vars_recursive function with various data types."""
+
+ # Set up test environment variables
+ test_env_vars = {
+ "TEST_HOST": "localhost",
+ "TEST_PORT": "8080",
+ "TEST_DB": "testdb",
+ "TEST_USER": "testuser",
+ }
+
+ for key, value in test_env_vars.items():
+ os.environ[key] = value
+
+ try:
+ # Test simple string expansion
+ simple_string = "${TEST_HOST}:${TEST_PORT}"
+ result = expand_env_vars_recursive(simple_string)
+ assert result == "localhost:8080"
+
+ # Test dictionary expansion
+ test_dict = {
+ "host": "${TEST_HOST}",
+ "port": "${TEST_PORT}",
+ "database": "${TEST_DB}",
+ "connection_string":
"postgresql://${TEST_USER}@${TEST_HOST}:${TEST_PORT}/${TEST_DB}",
+ "timeout": 30, # non-string should remain unchanged
+ "enabled": True, # non-string should remain unchanged
+ }
+
+ result_dict = expand_env_vars_recursive(test_dict)
+ expected_dict = {
+ "host": "localhost",
+ "port": "8080",
+ "database": "testdb",
+ "connection_string": "postgresql://testuser@localhost:8080/testdb",
+ "timeout": 30,
+ "enabled": True,
+ }
+ assert result_dict == expected_dict
+
+ # Test list expansion
+ test_list = [
+ "${TEST_HOST}",
+ {"nested_host": "${TEST_HOST}", "nested_port": "${TEST_PORT}"},
+ ["${TEST_USER}", "${TEST_DB}"],
+ 123, # non-string should remain unchanged
+ ]
+
+ result_list = expand_env_vars_recursive(test_list)
+ expected_list = [
+ "localhost",
+ {"nested_host": "localhost", "nested_port": "8080"},
+ ["testuser", "testdb"],
+ 123,
+ ]
+ assert result_list == expected_list
+
+ # Test undefined variables (should throw UnboundVariable)
+ with pytest.raises(UnboundVariable, match="'UNDEFINED_VAR: unbound
variable"):
+ expand_env_vars_recursive("${UNDEFINED_VAR}")
+
+ # Test mixed defined/undefined variables (should throw UnboundVariable)
+ with pytest.raises(UnboundVariable, match="'UNDEFINED_VAR: unbound
variable"):
+
expand_env_vars_recursive("prefix-${TEST_HOST}-middle-${UNDEFINED_VAR}-suffix")
+
+ finally:
+ # Clean up environment variables
+ for key in test_env_vars:
+ if key in os.environ:
+ del os.environ[key]
+
+
+def test_env_var_expansion_in_templates_and_tests():
+ """Test that environment variable expansion works in template and test
sections."""
+
+ # Set up test environment variables
+ test_env_vars = {
+ "CSV_DELIMITER": "$",
+ "CSV_QUOTE_CHAR": "!",
+ "CSV_FILENAME": "/tmp/test.csv",
+ }
+
+ for key, value in test_env_vars.items():
+ os.environ[key] = value
+
+ # Create a temporary config file with env var placeholders
+ config_content = """
+templates:
+ csv_template_1:
+ csv_options:
+ delimiter: "${CSV_DELIMITER}"
+
+ csv_template_2:
+ csv_options:
+ quote_char: '${CSV_QUOTE_CHAR}'
+
+tests:
+ expansion_test:
+ type: csv
+ file: ${CSV_FILENAME}
+ time_column: timestamp
+ metrics:
+ response_time:
+ column: response_ms
+ unit: ms
+ inherit: [csv_template_1, csv_template_2]
+"""
+
+ try:
+ with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml",
delete=False) as f:
+ f.write(config_content)
+ config_file_path = f.name
+
+ try:
+ # Load config and verify expansion worked
+ parser = create_config_parser()
+ args = parser.parse_args(["--config-file", config_file_path])
+ config = load_config_from_parser_args(args)
+
+ # Verify test was loaded
+ assert "expansion_test" in config.tests
+ test = config.tests["expansion_test"]
+ assert isinstance(test, CsvTestConfig)
+
+ # Verify that expansion worked
+ assert test.file == test_env_vars["CSV_FILENAME"]
+
+ # Verify that inheritance from templates worked with expanded
values
+ assert test.csv_options.delimiter == test_env_vars["CSV_DELIMITER"]
+ assert test.csv_options.quote_char ==
test_env_vars["CSV_QUOTE_CHAR"]
+
+ finally:
+ os.unlink(config_file_path)
+
+ finally:
+ # Clean up environment variables
+ for key in test_env_vars:
+ if key in os.environ:
+ del os.environ[key]
+
+
+def test_cli_precedence_over_env_vars():
+ """Test that CLI arguments take precedence over environment variables."""
+
+ # Set up environment variables
+ env_vars = {
+ "POSTGRES_HOSTNAME": "env-host.com",
+ "POSTGRES_PORT": "5433",
+ "POSTGRES_DATABASE": "env_db",
+ "SLACK_BOT_TOKEN": "env-slack-token",
+ }
+
+ for key, value in env_vars.items():
+ os.environ[key] = value
+
+ # Create a simple config file
+ config_content = """
+postgres:
+ hostname: config_host
+ port: 5432
+ database: config_db
+ username: config_user
+ password: config_pass
+
+slack:
+ token: config_slack_token
+"""
+
+ try:
+ with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml",
delete=False) as f:
+ f.write(config_content)
+ config_file_path = f.name
+
+ try:
+ # Test 1: Only environment variables (no CLI overrides)
+ config_env_only = load_config_from_file(config_file_path)
+
+ # Environment variables should override config file values
+ assert config_env_only.postgres.hostname == "env-host.com"
+ assert config_env_only.postgres.port == 5433
+ assert config_env_only.postgres.database == "env_db"
+ assert config_env_only.slack.bot_token == "env-slack-token"
+
+ # Values without env vars should use config file values
+ assert config_env_only.postgres.username == "config_user"
+ assert config_env_only.postgres.password == "config_pass"
+
+ # Test 2: CLI arguments should override environment variables
+ cli_overrides = [
+ "--postgres-hostname",
+ "cli-host.com",
+ "--postgres-port",
+ "5434",
+ "--slack-token",
+ "cli-slack-token",
+ ]
+
+ config_cli_override = load_config_from_file(
+ config_file_path, arg_overrides=cli_overrides
+ )
+
+ # CLI overrides should win
+ assert config_cli_override.postgres.hostname == "cli-host.com"
+ assert config_cli_override.postgres.port == 5434
+ assert config_cli_override.slack.bot_token == "cli-slack-token"
+
+ # Values without CLI override should still use env vars
+ assert config_cli_override.postgres.database == "env_db"
+ assert config_cli_override.postgres.username == "config_user"
+ assert config_cli_override.postgres.password == "config_pass"
+
+ finally:
+ os.unlink(config_file_path)
+
+ finally:
+ # Clean up environment variables
+ for key in env_vars:
+ if key in os.environ:
+ del os.environ[key]
diff --git a/uv.lock b/uv.lock
index a5beddf..f2b2833 100644
--- a/uv.lock
+++ b/uv.lock
@@ -10,11 +10,12 @@ resolution-markers = [
[[package]]
name = "apache-otava"
-version = "0.6.1"
+version = "0.7.0"
source = { editable = "." }
dependencies = [
{ name = "configargparse" },
{ name = "dateparser" },
+ { name = "expandvars" },
{ name = "google-cloud-bigquery", version = "3.30.0", source = { registry
= "https://pypi.org/simple"; }, marker = "python_full_version < '3.9'" },
{ name = "google-cloud-bigquery", version = "3.35.1", source = { registry
= "https://pypi.org/simple"; }, marker = "python_full_version >= '3.9'" },
{ name = "numpy" },
@@ -55,6 +56,7 @@ requires-dist = [
{ name = "autoflake", marker = "extra == 'dev'", specifier = ">=1.4" },
{ name = "configargparse", specifier = ">=1.7.1" },
{ name = "dateparser", specifier = ">=1.0.0" },
+ { name = "expandvars", specifier = ">=0.12.0" },
{ name = "flake8", marker = "extra == 'dev'", specifier = ">=4.0.1" },
{ name = "google-cloud-bigquery", specifier = ">=3.25.0" },
{ name = "isort", marker = "extra == 'dev'", specifier = ">=5.10.1" },
@@ -261,6 +263,15 @@ wheels = [
{ url =
"https://files.pythonhosted.org/packages/02/cc/b7e31358aac6ed1ef2bb790a9746ac2c69bcb3c8588b41616914eb106eaf/exceptiongroup-1.2.2-py3-none-any.whl";,
hash =
"sha256:3111b9d131c238bec2f8f516e123e14ba243563fb135d3fe885990585aa7795b", size
= 16453, upload-time = "2024-07-12T22:25:58.476Z" },
]
+[[package]]
+name = "expandvars"
+version = "1.1.2"
+source = { registry = "https://pypi.org/simple"; }
+sdist = { url =
"https://files.pythonhosted.org/packages/9c/64/a9d8ea289d663a44b346203a24bf798507463db1e76679eaa72ee6de1c7a/expandvars-1.1.2.tar.gz";,
hash =
"sha256:6c5822b7b756a99a356b915dd1267f52ab8a4efaa135963bd7f4bd5d368f71d7", size
= 70842, upload-time = "2025-09-12T10:55:20.929Z" }
+wheels = [
+ { url =
"https://files.pythonhosted.org/packages/7f/e6/79c43f7a55264e479a9fbf21ddba6a73530b3ea8439a8bb7fa5a281721af/expandvars-1.1.2-py3-none-any.whl";,
hash =
"sha256:d1652fe4e61914f5b88ada93aaedb396446f55ae4621de45c8cb9f66e5712526", size
= 7526, upload-time = "2025-09-12T10:55:18.779Z" },
+]
+
[[package]]
name = "filelock"
version = "3.16.1"
