Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Environment variables are now parsed for boolean, integer, array and object setting values #2637

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
17 changes: 16 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ duckdb-engine = { version = ">=0.9.4", python = "<4" }
fastjsonschema = ">=2.19.1"
pytest-benchmark = ">=4.0.0"
pytest-snapshot = ">=0.9.0"
pytest-subtests = ">=0.13.1"
pytz = ">=2022.2.1"
requests-mock = ">=1.10.0"
rfc3339-validator = ">=0.1.4"
Expand Down
45 changes: 31 additions & 14 deletions singer_sdk/configuration/_dict_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,27 @@
import logging
import os
import typing as t
import warnings
from pathlib import Path

from dotenv import find_dotenv
from dotenv.main import DotEnv

from singer_sdk.helpers._typing import is_string_array_type
from singer_sdk.helpers._util import read_json_file
from singer_sdk.helpers import _typing
from singer_sdk.helpers._util import load_json, read_json_file

logger = logging.getLogger(__name__)

TRUTHY = ("true", "1", "yes", "on")


def _parse_array(value: str) -> list[str]:
return load_json(value) # type: ignore[return-value]


def _legacy_parse_array_of_strings(value: str) -> list[str]:
return value.split(",")


def parse_environment_config(
config_schema: dict[str, t.Any],
Expand All @@ -29,9 +40,6 @@ def parse_environment_config(
dotenv_path: Path to a .env file. If None, will try to find one in increasingly
higher folders.

Raises:
ValueError: If an un-parsable setting is found.

Returns:
A configuration dictionary.
"""
Expand All @@ -43,7 +51,7 @@ def parse_environment_config(
logger.debug("Loading configuration from %s", dotenv_path)
DotEnv(dotenv_path).set_as_environment_variables()

for config_key in config_schema["properties"]:
for config_key, schema in config_schema.get("properties", {}).items():
env_var_name = prefix + config_key.upper().replace("-", "_")
if env_var_name in os.environ:
env_var_value = os.environ[env_var_name]
Expand All @@ -52,15 +60,24 @@ def parse_environment_config(
config_key,
env_var_name,
)
if is_string_array_type(config_schema["properties"][config_key]):
if env_var_value[0] == "[" and env_var_value[-1] == "]":
msg = (
"A bracketed list was detected in the environment variable "
f"'{env_var_name}'. This syntax is no longer supported. Please "
"remove the brackets and try again."
if _typing.is_integer_type(schema):
result[config_key] = int(env_var_value)
elif _typing.is_boolean_type(schema):
result[config_key] = env_var_value.lower() in TRUTHY
elif _typing.is_string_array_type(schema):
try:
result[config_key] = _parse_array(env_var_value)
except Exception: # noqa: BLE001
warnings.warn(
"Parsing array from deprecated string format 'x,y,z'",
DeprecationWarning,
stacklevel=2,
)
raise ValueError(msg)
result[config_key] = env_var_value.split(",")
result[config_key] = _legacy_parse_array_of_strings(env_var_value)
elif _typing.is_array_type(schema):
result[config_key] = _parse_array(env_var_value)
elif _typing.is_object_type(schema):
result[config_key] = load_json(env_var_value)
else:
result[config_key] = env_var_value
return result
Expand Down
97 changes: 69 additions & 28 deletions tests/core/configuration/test_dict_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import json
import typing as t
from pathlib import Path

import pytest
Expand All @@ -11,10 +12,23 @@
parse_environment_config,
)

if t.TYPE_CHECKING:
from pytest_subtests import SubTests

CONFIG_JSONSCHEMA = th.PropertiesList(
th.Property("prop1", th.StringType, required=True),
th.Property("prop2", th.StringType),
th.Property("prop3", th.ArrayType(th.StringType)),
th.Property("prop4", th.IntegerType),
th.Property("prop5", th.BooleanType),
th.Property("prop6", th.ArrayType(th.IntegerType)),
th.Property(
"prop7",
th.ObjectType(
th.Property("sub_prop1", th.StringType),
th.Property("sub_prop2", th.IntegerType),
),
),
).to_dict()


Expand All @@ -36,30 +50,66 @@ def config_file2(tmpdir) -> str:
return filepath


def test_get_env_var_config(monkeypatch: pytest.MonkeyPatch):
def test_get_env_var_config(monkeypatch: pytest.MonkeyPatch, subtests: SubTests):
"""Test settings parsing from environment variables."""
with monkeypatch.context() as m:
m.setenv("PLUGIN_TEST_PROP1", "hello")
m.setenv("PLUGIN_TEST_PROP3", "val1,val2")
m.setenv("PLUGIN_TEST_PROP4", "not-a-tap-setting")
m.setenv("PLUGIN_TEST_PROP3", '["val1","val2"]')
m.setenv("PLUGIN_TEST_PROP4", "123")
m.setenv("PLUGIN_TEST_PROP5", "TRUE")
m.setenv("PLUGIN_TEST_PROP6", "[1,2,3]")
m.setenv("PLUGIN_TEST_PROP7", '{"sub_prop1": "hello", "sub_prop2": 123}')
m.setenv("PLUGIN_TEST_PROP999", "not-a-tap-setting")
env_config = parse_environment_config(CONFIG_JSONSCHEMA, "PLUGIN_TEST_")
assert env_config["prop1"] == "hello"
assert env_config["prop3"] == ["val1", "val2"]
assert "PROP1" not in env_config
assert "prop2" not in env_config
assert "PROP2" not in env_config
assert "prop4" not in env_config
assert "PROP4" not in env_config

with subtests.test(msg="Parse string from environment"):
assert env_config["prop1"] == "hello"

with subtests.test(msg="Parse array from environment"):
assert env_config["prop3"] == ["val1", "val2"]

with subtests.test(msg="Parse integer from environment"):
assert env_config["prop4"] == 123

with subtests.test(msg="Parse boolean from environment"):
assert env_config["prop5"] is True

with subtests.test(msg="Parse array of integers from environment"):
assert env_config["prop6"] == [1, 2, 3]

with subtests.test(msg="Parse object from environment"):
assert env_config["prop7"] == {"sub_prop1": "hello", "sub_prop2": 123}

with subtests.test(msg="Ignore non-tap setting"):
missing_props = {"PROP1", "prop2", "PROP2", "prop999", "PROP999"}
assert not set.intersection(missing_props, env_config)

m.setenv("PLUGIN_TEST_PROP3", "val1,val2")
with subtests.test(msg="Legacy array parsing"), pytest.warns(
DeprecationWarning
):
parsed = parse_environment_config(CONFIG_JSONSCHEMA, "PLUGIN_TEST_")
assert parsed["prop3"] == ["val1", "val2"]

no_env_config = parse_environment_config(CONFIG_JSONSCHEMA, "PLUGIN_TEST_")
assert "prop1" not in no_env_config
assert "PROP1" not in env_config
assert "prop2" not in no_env_config
assert "PROP2" not in env_config
assert "prop3" not in no_env_config
assert "PROP3" not in env_config
assert "prop4" not in no_env_config
assert "PROP4" not in env_config
missing_props = {
"prop1",
"PROP1",
"prop2",
"PROP2",
"prop3",
"PROP3",
"prop4",
"PROP4",
"prop5",
"PROP5",
"prop6",
"PROP6",
"prop999",
"PROP999",
}
with subtests.test(msg="Ignore missing environment variables"):
assert not set.intersection(missing_props, no_env_config)


def test_get_dotenv_config(tmp_path: Path):
Expand All @@ -74,15 +124,6 @@ def test_get_dotenv_config(tmp_path: Path):
assert dotenv_config["prop1"] == "hello"


def test_get_env_var_config_not_parsable(monkeypatch: pytest.MonkeyPatch):
"""Test settings parsing from environment variables with a non-parsable value."""
with monkeypatch.context() as m:
m.setenv("PLUGIN_TEST_PROP1", "hello")
m.setenv("PLUGIN_TEST_PROP3", '["repeated"]')
with pytest.raises(ValueError, match="A bracketed list was detected"):
parse_environment_config(CONFIG_JSONSCHEMA, "PLUGIN_TEST_")


def test_merge_config_sources(
config_file1,
config_file2,
Expand All @@ -91,7 +132,7 @@ def test_merge_config_sources(
"""Test merging multiple configuration sources."""
with monkeypatch.context() as m:
m.setenv("PLUGIN_TEST_PROP1", "from-env")
m.setenv("PLUGIN_TEST_PROP4", "not-a-tap-setting")
m.setenv("PLUGIN_TEST_PROP999", "not-a-tap-setting")
config = merge_config_sources(
[config_file1, config_file2, "ENV"],
CONFIG_JSONSCHEMA,
Expand Down
Loading