From 3ddcc64b13e1bc3f0d1176bdc99cf36e7a9be3e8 Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:01:57 -0500 Subject: [PATCH 1/6] Add config source interface and environment variable source for config resolution (#640) * Add config source interface and environment variable source --- .../src/smithy_aws_core/config/__init__.py | 7 ++ .../src/smithy_aws_core/config/sources.py | 35 ++++++++ .../tests/unit/config/test_sources.py | 86 +++++++++++++++++++ .../src/smithy_core/interfaces/config.py | 27 ++++++ 4 files changed, 155 insertions(+) create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/config/__init__.py create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/config/sources.py create mode 100644 packages/smithy-aws-core/tests/unit/config/test_sources.py create mode 100644 packages/smithy-core/src/smithy_core/interfaces/config.py diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/__init__.py b/packages/smithy-aws-core/src/smithy_aws_core/config/__init__.py new file mode 100644 index 000000000..e4ac5fd5f --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/__init__.py @@ -0,0 +1,7 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from .sources import EnvironmentSource + +__all__ = [ + "EnvironmentSource", +] diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py b/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py new file mode 100644 index 000000000..f5e5195de --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py @@ -0,0 +1,35 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +import os + + +class EnvironmentSource: + """Configuration from environment variables.""" + + def __init__(self, prefix: str = "AWS_"): + """Initialize the EnvironmentSource with environment variable prefix. + + :param prefix: Prefix for environment variables (default: 'AWS_') + """ + self._prefix = prefix + + @property + def name(self) -> str: + """Returns the source name.""" + return "environment" + + def get(self, key: str) -> str | None: + """Returns a configuration value from environment variables. + + The key is transformed to uppercase and prefixed (e.g., 'region' → 'AWS_REGION'). + + :param key: The standard configuration key (e.g., 'region', 'retry_mode'). + + :returns: The value from the environment variable (or empty string if set to empty), + or None if the variable is not set. + """ + env_var = f"{self._prefix}{key.upper()}" + config_value = os.environ.get(env_var) + if config_value is None: + return None + return config_value.strip() diff --git a/packages/smithy-aws-core/tests/unit/config/test_sources.py b/packages/smithy-aws-core/tests/unit/config/test_sources.py new file mode 100644 index 000000000..5db84f38e --- /dev/null +++ b/packages/smithy-aws-core/tests/unit/config/test_sources.py @@ -0,0 +1,86 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +import os +from unittest.mock import patch + +from smithy_aws_core.config.sources import EnvironmentSource + + +class TestEnvironmentSource: + def test_source_name(self): + source = EnvironmentSource() + assert source.name == "environment" + + def test_get_region_from_aws_region(self): + with patch.dict(os.environ, {"AWS_REGION": "us-west-2"}, clear=True): + source = EnvironmentSource() + value = source.get("region") + assert value == "us-west-2" + + def test_get_returns_none_when_env_var_not_set(self): + with patch.dict(os.environ, {}, clear=True): + source = EnvironmentSource() + value = source.get("region") + assert value is None + + def test_get_returns_none_for_unknown_key(self): + source = EnvironmentSource() + value = source.get("unknown_config_key") + assert value is None + + def test_get_handles_empty_string_env_var(self): + with patch.dict(os.environ, {"AWS_REGION": ""}, clear=True): + source = EnvironmentSource() + value = source.get("region") + assert value == "" + + def test_get_handles_whitespace_env_var(self): + with patch.dict(os.environ, {"AWS_REGION": " us-west-2 "}, clear=True): + source = EnvironmentSource() + value = source.get("region") + # Whitespaces should be stripped + assert value == "us-west-2" + + def test_get_handles_whole_whitespace_env_var(self): + with patch.dict(os.environ, {"AWS_REGION": " "}, clear=True): + source = EnvironmentSource() + value = source.get("region") + # Whitespaces should be stripped + assert value == "" + + def test_multiple_keys_with_different_env_vars(self): + env_vars = {"AWS_REGION": "eu-west-1", "AWS_RETRY_MODE": "standard"} + with patch.dict(os.environ, env_vars, clear=True): + source = EnvironmentSource() + + region = source.get("region") + retry_mode = source.get("retry_mode") + + assert region == "eu-west-1" + assert retry_mode == "standard" + + def test_get_is_idempotent(self): + with patch.dict(os.environ, {"AWS_REGION": "ap-south-1"}, clear=True): + source = EnvironmentSource() + # Calling get on source multiple times should return the same value + value1 = source.get("region") + value2 = source.get("region") + value3 = source.get("region") + + assert value1 == value2 == value3 == "ap-south-1" + + def test_source_does_not_cache_env_vars(self): + source = EnvironmentSource() + + # First read + with patch.dict(os.environ, {"AWS_REGION": "us-east-1"}, clear=True): + value1 = source.get("region") + assert value1 == "us-east-1" + + # Environment changes + with patch.dict(os.environ, {"AWS_REGION": "us-west-2"}, clear=False): + value2 = source.get("region") + assert value2 == "us-west-2" + + # Source reads from os.environ and not from cache + assert value1 != value2 diff --git a/packages/smithy-core/src/smithy_core/interfaces/config.py b/packages/smithy-core/src/smithy_core/interfaces/config.py new file mode 100644 index 000000000..add120a02 --- /dev/null +++ b/packages/smithy-core/src/smithy_core/interfaces/config.py @@ -0,0 +1,27 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from typing import Any, Protocol, runtime_checkable + + +@runtime_checkable +class ConfigSource(Protocol): + """Protocol for configuration sources that provide values from various locations + like environment variables and configuration files. + """ + + @property + def name(self) -> str: + """Returns a string identifying the source. + + :returns: A string identifier for this source. + """ + ... + + def get(self, key: str) -> Any | None: + """Returns a configuration value from the source. + + :param key: The configuration key to retrieve (e.g., 'region') + + :returns: The value associated with the key, or None if not found. + """ + ... From 318519ffcbe86d5e826c104b7a1d1bfe85e93b58 Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Mon, 23 Feb 2026 18:11:27 -0500 Subject: [PATCH 2/6] Add ConfigResolver with necessary tests (#641) * Add a ConfigResolver that goes through the list of sources to resolve config value --- .../src/smithy_core/config/__init__.py | 8 ++ .../src/smithy_core/config/resolver.py | 36 ++++++ .../smithy-core/tests/unit/config/__init__.py | 2 + .../tests/unit/config/test_resolver.py | 112 ++++++++++++++++++ 4 files changed, 158 insertions(+) create mode 100644 packages/smithy-core/src/smithy_core/config/__init__.py create mode 100644 packages/smithy-core/src/smithy_core/config/resolver.py create mode 100644 packages/smithy-core/tests/unit/config/__init__.py create mode 100644 packages/smithy-core/tests/unit/config/test_resolver.py diff --git a/packages/smithy-core/src/smithy_core/config/__init__.py b/packages/smithy-core/src/smithy_core/config/__init__.py new file mode 100644 index 000000000..2a8cc831d --- /dev/null +++ b/packages/smithy-core/src/smithy_core/config/__init__.py @@ -0,0 +1,8 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +from .resolver import ConfigResolver + +__all__ = [ + "ConfigResolver", +] diff --git a/packages/smithy-core/src/smithy_core/config/resolver.py b/packages/smithy-core/src/smithy_core/config/resolver.py new file mode 100644 index 000000000..7a82ca2b6 --- /dev/null +++ b/packages/smithy-core/src/smithy_core/config/resolver.py @@ -0,0 +1,36 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from collections.abc import Sequence +from typing import Any + +from smithy_core.interfaces.config import ConfigSource + + +class ConfigResolver: + """Resolves configuration values from multiple sources. + + The resolver iterates through sources in precedence order, returning + the first non-None value found for a given configuration key. + """ + + def __init__(self, sources: Sequence[ConfigSource]) -> None: + """Initialize the resolver with sources in precedence order. + + :param sources: List of configuration sources in precedence order. The first + source in the list has the highest priority. + """ + self._sources = sources + + def get(self, key: str) -> tuple[Any, str | None]: + """Resolve a configuration value from sources by iterating through them in precedence order. + + :param key: The configuration key to resolve (e.g., 'retry_mode') + + :returns: A tuple of (value, source_name). If no source provides a value, + returns (None, None). + """ + for source in self._sources: + value = source.get(key) + if value is not None: + return (value, source.name) + return (None, None) diff --git a/packages/smithy-core/tests/unit/config/__init__.py b/packages/smithy-core/tests/unit/config/__init__.py new file mode 100644 index 000000000..04f8b7b76 --- /dev/null +++ b/packages/smithy-core/tests/unit/config/__init__.py @@ -0,0 +1,2 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 diff --git a/packages/smithy-core/tests/unit/config/test_resolver.py b/packages/smithy-core/tests/unit/config/test_resolver.py new file mode 100644 index 000000000..bf016e1d2 --- /dev/null +++ b/packages/smithy-core/tests/unit/config/test_resolver.py @@ -0,0 +1,112 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from typing import Any + +from smithy_core.config.resolver import ConfigResolver + + +class StubSource: + """A simple ConfigSource implementation for testing. + + Returns values from a provided dictionary, or None if the key + is not present. + """ + + def __init__(self, source_name: str, data: dict[str, Any] | None = None): + self._name = source_name + self._data = data or {} + + @property + def name(self) -> str: + return self._name + + def get(self, key: str) -> Any | None: + return self._data.get(key) + + +class TestConfigResolver: + def test_returns_value_from_single_source(self): + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + + result = resolver.get("region") + + assert result == ("us-west-2", "environment") + + def test_returns_None_when_source_has_no_value(self): + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + + result = resolver.get("region") + + assert result == (None, None) + + def test_returns_None_with_empty_source_list(self): + resolver = ConfigResolver(sources=[]) + + result = resolver.get("region") + + assert result == (None, None) + + def test_first_source_takes_precedence(self): + first_priority_source = StubSource("source_one", {"region": "us-east-1"}) + second_priority_source = StubSource("source_two", {"region": "eu-west-1"}) + resolver = ConfigResolver( + sources=[first_priority_source, second_priority_source] + ) + + result = resolver.get("region") + + assert result == ("us-east-1", "source_one") + + def test_skips_source_returning_none_and_uses_next(self): + empty_source = StubSource("source_one", {}) + fallback_source = StubSource("source_two", {"region": "ap-south-1"}) + resolver = ConfigResolver(sources=[empty_source, fallback_source]) + + result = resolver.get("region") + + assert result == ("ap-south-1", "source_two") + + def test_resolves_different_keys_from_different_sources(self): + instance = StubSource("source_one", {"region": "us-west-2"}) + environment = StubSource("source_two", {"retry_mode": "adaptive"}) + resolver = ConfigResolver(sources=[instance, environment]) + + region = resolver.get("region") + retry_mode = resolver.get("retry_mode") + + assert region == ("us-west-2", "source_one") + assert retry_mode == ("adaptive", "source_two") + + def test_returns_non_string_values(self): + source = StubSource( + "default", + { + "max_retries": 3, + "use_ssl": True, + }, + ) + resolver = ConfigResolver(sources=[source]) + + assert resolver.get("max_retries") == (3, "default") + assert resolver.get("use_ssl") == (True, "default") + + def test_get_is_idempotent(self): + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + + result1 = resolver.get("region") + result2 = resolver.get("region") + result3 = resolver.get("region") + + assert result1 == result2 == result3 == ("us-west-2", "environment") + + def test_treats_empty_string_as_valid_value(self): + source = StubSource("test", {"region": ""}) + resolver = ConfigResolver(sources=[source]) + + value, source_name = resolver.get("region") + + assert value == "" + assert source_name == "test" From eb22199e0743cd7767c90fdd0f0070410677acf5 Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:47:19 -0500 Subject: [PATCH 3/6] Add a config property descriptor along with a custom resolver and validators (#642) * Add a config property descriptor along with a custom resolver and validators --- .../config/custom_resolvers.py | 50 ++++ .../src/smithy_aws_core/config/validators.py | 126 ++++++++ .../tests/unit/config/test_custom_resolver.py | 96 ++++++ .../tests/unit/config/test_validators.py | 51 ++++ .../src/smithy_core/config/property.py | 106 +++++++ .../tests/unit/config/test_property.py | 277 ++++++++++++++++++ 6 files changed, 706 insertions(+) create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/config/validators.py create mode 100644 packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py create mode 100644 packages/smithy-aws-core/tests/unit/config/test_validators.py create mode 100644 packages/smithy-core/src/smithy_core/config/property.py create mode 100644 packages/smithy-core/tests/unit/config/test_property.py diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py new file mode 100644 index 000000000..675570695 --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py @@ -0,0 +1,50 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +from smithy_core.config.resolver import ConfigResolver +from smithy_core.retries import RetryStrategyOptions + +from smithy_aws_core.config.validators import validate_max_attempts, validate_retry_mode + + +def resolve_retry_strategy( + resolver: ConfigResolver, +) -> tuple[RetryStrategyOptions | None, str | None]: + """Resolve retry strategy from multiple config keys. + + Resolves both retry_mode and max_attempts from sources and constructs + a RetryStrategyOptions object. This allows the retry strategy to be + configured from multiple sources. Example: retry_mode from config file and + max_attempts from environment variables. + + :param resolver: The config resolver to use for resolution + + :returns: Tuple of (RetryStrategyOptions, source_name) if both retry_mode and max_attempts + are resolved. Returns (None, None) if both values are missing. + + For mixed sources, the source name includes both component sources: + "retry_mode=environment, max_attempts=config_file" + """ + + retry_mode, mode_source = resolver.get("retry_mode") + + max_attempts, attempts_source = resolver.get("max_attempts") + + if retry_mode is None and max_attempts is None: + return None, None + + if retry_mode is not None: + retry_mode = validate_retry_mode(retry_mode, mode_source) + + if max_attempts is not None: + max_attempts = validate_max_attempts(max_attempts, attempts_source) + + options = RetryStrategyOptions( + retry_mode=retry_mode, # type: ignore + max_attempts=max_attempts, # Can be None because strategy will use its default + ) + + # Construct mixed source string showing where each component came from + source = f"retry_mode={mode_source or 'default'}, max_attempts={attempts_source or 'default'}" + + return (options, source) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py new file mode 100644 index 000000000..65f9a536f --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py @@ -0,0 +1,126 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +import re +from typing import Any, get_args + +from smithy_core.interfaces.retries import RetryStrategy +from smithy_core.retries import RetryStrategyOptions, RetryStrategyType + + +class ConfigValidationError(ValueError): + """Raised when a configuration value fails validation.""" + + def __init__(self, key: str, value: Any, reason: str, source: str | None = None): + self.key = key + self.value = value + self.reason = reason + self.source = source + + msg = f"Invalid value for '{key}': {value!r}. {reason}" + if source: + msg += f" (from source: {source})" + super().__init__(msg) + + +def validate_region(region: str, source: str | None = None) -> str: + """Validate region name format. + + :param region: The value to validate + :param source: The config source that provided this value + + :returns: The validated value + + :raises ConfigValidationError: If the value format is invalid + """ + pattern = r"^(?![0-9]+$)(?!-)[a-zA-Z0-9-]{1,63}(? str: + """Validate retry mode. + + Valid values: 'standard', 'simple' + + :param retry_mode: The retry mode value to validate + :param source: The source that provided this value + + :returns: The validated retry mode string + + :raises: ConfigValidationError: If the retry mode is invalid + """ + + valid_modes = get_args(RetryStrategyType) + + if retry_mode not in valid_modes: + raise ConfigValidationError( + "retry_mode", + retry_mode, + f"retry_mode must be one of {valid_modes}, got {retry_mode}", + source, + ) + + return retry_mode + + +def validate_max_attempts(max_attempts: str | int, source: str | None = None) -> int: + """Validate and convert max_attempts to integer. + + :param max_attempts: The max attempts value (string or int) + :param source: The source that provided this value + + :returns: The validated max_attempts as an integer + + :raises ConfigValidationError: If the value is less than 1 or cannot be converted to an integer + """ + try: + max_attempts = int(max_attempts) + except (ValueError, TypeError): + raise ConfigValidationError( + "max_attempts", + max_attempts, + f"max_attempts must be a number, got {type(max_attempts).__name__}", + source, + ) + + if max_attempts < 1: + raise ConfigValidationError( + "max_attempts", + max_attempts, + f"max_attempts must be a positive integer, got {max_attempts}", + source, + ) + + return max_attempts + + +def validate_retry_strategy( + value: Any, source: str | None = None +) -> RetryStrategy | RetryStrategyOptions: + """Validate retry strategy configuration. + + :param value: The retry strategy value to validate + :param source: The source that provided this value + + :returns: The validated retry strategy (RetryStrategy or RetryStrategyOptions) + + :raises: ConfigValidationError: If the value is not a valid retry strategy type + """ + + if isinstance(value, RetryStrategy | RetryStrategyOptions): + return value + + raise ConfigValidationError( + "retry_strategy", + value, + f"retry_strategy must be RetryStrategy or RetryStrategyOptions, got {type(value).__name__}", + source, + ) diff --git a/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py new file mode 100644 index 000000000..c9e7d23e9 --- /dev/null +++ b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py @@ -0,0 +1,96 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +from typing import Any + +from smithy_aws_core.config.custom_resolvers import resolve_retry_strategy +from smithy_core.config.resolver import ConfigResolver +from smithy_core.retries import RetryStrategyOptions + + +class StubSource: + """A simple ConfigSource implementation for testing.""" + + def __init__(self, source_name: str, data: dict[str, Any] | None = None) -> None: + self._name = source_name + self._data = data or {} + + @property + def name(self) -> str: + return self._name + + def get(self, key: str) -> Any | None: + return self._data.get(key) + + +class TestResolveCustomResolverRetryStrategy: + """Test suite for complex configuration resolution""" + + def test_resolves_from_both_values(self) -> None: + # When both retry mode and max attempts are set + # It should use source names for both values + source = StubSource( + "environment", {"retry_mode": "standard", "max_attempts": "3"} + ) + resolver = ConfigResolver(sources=[source]) + + result, source_name = resolve_retry_strategy(resolver) + + assert isinstance(result, RetryStrategyOptions) + assert result.retry_mode == "standard" + assert result.max_attempts == 3 + assert source_name == "retry_mode=environment, max_attempts=environment" + + def test_tracks_different_sources_for_each_component(self) -> None: + source1 = StubSource("environment", {"retry_mode": "standard"}) + source2 = StubSource("config_file", {"max_attempts": "5"}) + resolver = ConfigResolver(sources=[source1, source2]) + + result, source_name = resolve_retry_strategy(resolver) + + assert isinstance(result, RetryStrategyOptions) + assert result.retry_mode == "standard" + assert result.max_attempts == 5 + assert source_name == "retry_mode=environment, max_attempts=config_file" + + def test_converts_max_attempts_string_to_int(self) -> None: + source = StubSource( + "environment", {"max_attempts": "10", "retry_mode": "standard"} + ) + resolver = ConfigResolver(sources=[source]) + + result, _ = resolve_retry_strategy(resolver) + + assert isinstance(result, RetryStrategyOptions) + assert result.max_attempts == 10 + assert isinstance(result.max_attempts, int) + + def test_returns_strategy_when_only_retry_mode_set(self) -> None: + source = StubSource("environment", {"retry_mode": "standard"}) + resolver = ConfigResolver(sources=[source]) + + result, source_name = resolve_retry_strategy(resolver) + + assert isinstance(result, RetryStrategyOptions) + assert result.retry_mode == "standard" + assert result.max_attempts is None + assert source_name == "retry_mode=environment, max_attempts=default" + + def test_returns_strategy_when_only_max_attempts_set(self) -> None: + source = StubSource("environment", {"max_attempts": "5"}) + resolver = ConfigResolver(sources=[source]) + + result, source_name = resolve_retry_strategy(resolver) + + assert isinstance(result, RetryStrategyOptions) + assert result.max_attempts == 5 + assert source_name == "retry_mode=default, max_attempts=environment" + + def test_returns_none_when_both_values_missing(self) -> None: + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + + result, source_name = resolve_retry_strategy(resolver) + + assert result is None + assert source_name is None diff --git a/packages/smithy-aws-core/tests/unit/config/test_validators.py b/packages/smithy-aws-core/tests/unit/config/test_validators.py new file mode 100644 index 000000000..2ce809c90 --- /dev/null +++ b/packages/smithy-aws-core/tests/unit/config/test_validators.py @@ -0,0 +1,51 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from typing import Any + +import pytest +from smithy_aws_core.config.validators import ( + ConfigValidationError, + validate_max_attempts, + validate_region, + validate_retry_mode, +) + + +class TestValidators: + @pytest.mark.parametrize("region", ["us-east-1", "eu-west-1", "ap-south-1"]) + def test_validate_region_accepts_valid_values(self, region: str) -> None: + assert validate_region(region) == region + + @pytest.mark.parametrize("invalid", ["-invalid", "-east", "12345", ""]) + def test_validate_region_rejects_invalid_values(self, invalid: str) -> None: + with pytest.raises(ConfigValidationError): + validate_region(invalid) + + @pytest.mark.parametrize("mode", ["standard", "simple"]) + def test_validate_retry_mode_accepts_valid_values(self, mode: str) -> None: + assert validate_retry_mode(mode) == mode + + @pytest.mark.parametrize("invalid_mode", ["some_retry", "some_retry_one", ""]) + def test_validate_retry_mode_rejects_invalid_values( + self, invalid_mode: str + ) -> None: + with pytest.raises(ConfigValidationError): + validate_retry_mode(invalid_mode) + + @pytest.mark.parametrize("invalid_max_attempts", ["abcd", 0, -1]) + def test_validate_invalid_max_attempts_raises_error( + self, invalid_max_attempts: Any + ) -> None: + with pytest.raises( + ConfigValidationError, + match=r"(max_attempts must be a number|max_attempts must be a positive integer)", + ): + validate_max_attempts(invalid_max_attempts) + + def test_invalid_retry_mode_error_message(self) -> None: + with pytest.raises(ConfigValidationError) as exc_info: + validate_retry_mode("random_mode") + assert ( + "Invalid value for 'retry_mode': 'random_mode'. retry_mode must be one " + "of ('simple', 'standard'), got random_mode" in str(exc_info.value) + ) diff --git a/packages/smithy-core/src/smithy_core/config/property.py b/packages/smithy-core/src/smithy_core/config/property.py new file mode 100644 index 000000000..71a3f413c --- /dev/null +++ b/packages/smithy-core/src/smithy_core/config/property.py @@ -0,0 +1,106 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from collections.abc import Callable +from typing import Any + +from smithy_core.config.resolver import ConfigResolver + + +class ConfigProperty: + """Descriptor for config properties with resolution, caching, and validation. + + This descriptor handles: + - Lazy resolution from sources (only on first access) + - Custom resolution for variables requiring complex resolution + - Caching of resolved values + - Source tracking for provenance + - Validation of values + + Example: + class Config: + region = ConfigProperty('region', validator=validate_region) + + def __init__(self): + self._resolver = ConfigResolver(sources=[...]) + """ + + def __init__( + self, + key: str, + validator: Callable[[Any, str | None], Any] | None = None, + resolver_func: Callable[[ConfigResolver], tuple[Any, str | None]] | None = None, + default_value: Any = None, + ): + """Initialize config property descriptor. + + :param key: The configuration key (e.g., 'region') + :param validator: Optional validation function that takes (value, source) + and returns validated value or raises an exception + :param resolver_func: Optional custom resolver function for complex resolution. + Takes a ConfigResolver and returns (value, source) tuple. + """ + self.key = key + self.validator = validator + self.resolver_func = resolver_func + # Cache attribute name in instance __dict__ (e.g., "_cache_region") + self.cache_attr = f"_cache_{key}" + self.default_value = default_value + + def __get__(self, obj: Any, objtype: type | None = None) -> Any: + """Get the config value with lazy resolution and caching. + + On first access, the property checks if the value is already cached. If not, it resolves + the value from sources using resolver. When a validator is provided, the resolved value + is validated before use. Finally, the property caches the (value, source) tuple. On + subsequent accesses, it returns the cached value. + + :param obj: The Config instance + :param objtype: The Config class + + :returns: The resolved and validated config value + """ + # If accessed on class instead of instance, return descriptor itself + if obj is None: + return self + + cached = getattr(obj, self.cache_attr, None) + if cached is not None: + return cached[ + 0 + ] # Return value from tuple (value, source) if already cached + + # If not cached, use a resolver to go through the sources to get (value, source) + # For complex config resolutions, use a custom resolver function to resolve values + if self.resolver_func: + value, source = self.resolver_func(obj._resolver) + else: + value, source = obj._resolver.get(self.key) + + if value is None: + value = self.default_value + source = "default" + + if self.validator: + value = self.validator(value, source) + + setattr(obj, self.cache_attr, (value, source)) + return value + + def __set__(self, obj: Any, value: Any) -> None: + """Set the config value (called during __init__ or after). + + When a config value is set, the property validates the new value if a validator is provided, then + updates the cached (value, source) tuple. The source is marked as 'instance' if the value + is set during __init__, or 'in-code' if set later. + + :param obj: The Config instance + :param value: The new value to set + """ + # Determine source based on when the value was set + # If cache already exists, it means it was not set during initialization + # In that case source will be set to in-code + source = "in-code" if hasattr(obj, self.cache_attr) else "instance" + if self.validator: + value = self.validator(value, source) + + setattr(obj, self.cache_attr, (value, source)) diff --git a/packages/smithy-core/tests/unit/config/test_property.py b/packages/smithy-core/tests/unit/config/test_property.py new file mode 100644 index 000000000..88ae990a6 --- /dev/null +++ b/packages/smithy-core/tests/unit/config/test_property.py @@ -0,0 +1,277 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +from collections.abc import Callable +from typing import Any, NoReturn + +import pytest +from smithy_core.config.property import ConfigProperty +from smithy_core.config.resolver import ConfigResolver + + +class StubSource: + """A simple ConfigSource implementation for testing.""" + + def __init__(self, source_name: str, data: dict[str, Any] | None = None) -> None: + self._name = source_name + self._data = data or {} + + @property + def name(self) -> str: + return self._name + + def get(self, key: str) -> Any | None: + return self._data.get(key) + + +class StubConfig: + """A minimal Config class for testing ConfigProperty descriptor.""" + + region = ConfigProperty("region") + retry_mode = ConfigProperty("retry_mode") + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + +class TestConfigPropertyDescriptor: + def test_resolves_value_from_resolver_on_first_access(self) -> None: + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + result = config.region + + assert result == "us-west-2" + + def test_caches_resolved_value(self) -> None: + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + # First access + result1 = config.region + # Second access + result2 = config.region + + assert result1 == result2 == "us-west-2" + # Verify it's cached + assert hasattr(config, "_cache_region") + + def test_uses_default_value_when_unresolved(self) -> None: + class ConfigWithDefault: + region = ConfigProperty("region", default_value="us-east-1") + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithDefault(resolver) + + result = config.region + + assert result == "us-east-1" + assert getattr(config, "_cache_region") == ("us-east-1", "default") + + def test_different_properties_resolve_independently(self) -> None: + source = StubSource( + "environment", {"region": "us-west-2", "retry_mode": "adaptive"} + ) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + region = config.region + retry_mode = config.retry_mode + + assert region == "us-west-2" + assert retry_mode == "adaptive" + + +class TestConfigPropertyValidation: + """Test suite for ConfigProperty validation behavior.""" + + def _create_config_with_validator( + self, validator: Callable[[Any, str | None], Any] + ) -> type[Any]: + """Helper to create a config class with a specific validator.""" + + class ConfigWithValidator: + region = ConfigProperty("region", validator=validator) + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + return ConfigWithValidator + + def test_calls_validator_on_resolution(self) -> None: + call_log: list[tuple[Any, str | None]] = [] + + def mock_validator(value: Any, source: str | None) -> Any: + call_log.append((value, source)) + return value + + ConfigWithValidator = self._create_config_with_validator(mock_validator) + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithValidator(resolver) + + result = config.region + + assert result == "us-west-2" + assert len(call_log) == 1 + assert call_log[0] == ("us-west-2", "environment") + + def test_validator_exception_propagates(self) -> None: + def failing_validator(value: Any, source: str | None) -> NoReturn: + raise ValueError("Invalid value") + + ConfigWithValidator = self._create_config_with_validator(failing_validator) + source = StubSource("environment", {"region": "invalid-region-123"}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithValidator(resolver) + + with pytest.raises(ValueError, match="Invalid value"): + config.region + + def test_validator_not_called_on_cached_access(self) -> None: + call_count = 0 + + def counting_validator(value: Any, source: str | None) -> Any: + nonlocal call_count + call_count += 1 + return value + + ConfigWithValidator = self._create_config_with_validator(counting_validator) + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithValidator(resolver) + + # Multiple accesses + config.region + config.region + config.region + + # Only the first call accessed the validator + assert call_count == 1 # Validator called only once + + +class TestConfigPropertySetter: + """Test suite for ConfigProperty setter behavior.""" + + def test_set_value_marks_source_as_instance(self) -> None: + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + config.region = "eu-west-1" + + # Check the cached tuple + assert getattr(config, "_cache_region") == ("eu-west-1", "instance") + + def test_value_set_after_resolution_marks_source_as_in_code(self) -> None: + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + # First access triggers resolution from environment source + config.region + + # Modify after resolution + config.region = "eu-west-1" + + # Verify the new value is returned + assert config.region == "eu-west-1" + # Verify source is marked as 'in-code' + # Any config value modified after initialization will have 'in-code' for source + assert getattr(config, "_cache_region") == ("eu-west-1", "in-code") + + def test_validator_is_called_when_setting_values(self) -> None: + call_log: list[tuple[Any, str | None]] = [] + + def mock_validator(value: Any, source: str | None) -> Any: + call_log.append((value, source)) + return value + + class ConfigWithValidator: + region = ConfigProperty("region", validator=mock_validator) + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithValidator(resolver) + + config.region = "us-west-2" + + assert config.region == "us-west-2" + assert len(call_log) == 1 + assert call_log[0] == ("us-west-2", "instance") + + def test_validator_throws_exception_when_setting_invalid_value(self) -> None: + def mock_failing_validation(value: Any, source: str | None) -> NoReturn: + raise ValueError("Invalid value") + + class ConfigWithValidator: + region = ConfigProperty("region", validator=mock_failing_validation) + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithValidator(resolver) + + with pytest.raises(ValueError, match="Invalid value"): + config.region = "some-invalid-2" + + def test_set_overrides_resolved_value(self) -> None: + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + # First access resolves from environment + assert config.region == "us-west-2" + + # Setting overrides + config.region = "eu-west-1" + + assert config.region == "eu-west-1" + + +class TestConfigPropertyCaching: + """Test suite for ConfigProperty caching implementation details.""" + + def test_cache_stores_value_and_source_as_tuple(self) -> None: + source = StubSource("environment", {"region": "us-west-2"}) + resolver = ConfigResolver(sources=[source]) + config = StubConfig(resolver) + + config.region + + cached: Any = getattr(config, "_cache_region") + assert cached == ("us-west-2", "environment") + + def test_validator_called_on_default_value(self) -> None: + call_log: list[tuple[Any, str | None]] = [] + + def mock_validator(value: Any, source: str | None) -> Any: + call_log.append((value, source)) + return value + + class ConfigWithDefault: + region = ConfigProperty( + "region", default_value="us-default-1", validator=mock_validator + ) + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithDefault(resolver) + + config.region + + assert call_log == [("us-default-1", "default")] From 3cbe664068889d556c4b390f7067cc5bd0e52f60 Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Thu, 5 Mar 2026 10:58:01 -0500 Subject: [PATCH 4/6] Update the codegen files that generate config.py to support config resolution. (#646) * Update the codegen files that generate config.py to support config resolution. --- .../aws/codegen/AwsAuthIntegration.java | 2 + .../python/aws/codegen/AwsConfiguration.java | 41 ++- .../smithy/python/codegen/ConfigProperty.java | 91 ++++++- .../codegen/generators/ConfigGenerator.java | 244 +++++++++++++++--- .../config/custom_resolvers.py | 4 +- .../src/smithy_aws_core/config/validators.py | 22 +- .../tests/unit/config/test_custom_resolver.py | 3 + .../tests/unit/config/test_validators.py | 4 +- 8 files changed, 369 insertions(+), 42 deletions(-) diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsAuthIntegration.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsAuthIntegration.java index 8358c54be..d2b678751 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsAuthIntegration.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsAuthIntegration.java @@ -5,6 +5,7 @@ package software.amazon.smithy.python.aws.codegen; import static software.amazon.smithy.python.aws.codegen.AwsConfiguration.REGION; +import static software.amazon.smithy.python.aws.codegen.AwsConfiguration.RETRY_STRATEGY; import java.util.List; import java.util.Set; @@ -64,6 +65,7 @@ public List getClientPlugins(GenerationContext context) { .nullable(true) .build()) .addConfigProperty(REGION) + .addConfigProperty(RETRY_STRATEGY) .addConfigProperty(ConfigProperty.builder() .name("aws_access_key_id") .type(Symbol.builder().name("str").build()) diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsConfiguration.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsConfiguration.java index 5d3ba035f..01529a4ff 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsConfiguration.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsConfiguration.java @@ -17,8 +17,47 @@ private AwsConfiguration() {} public static final ConfigProperty REGION = ConfigProperty.builder() .name("region") - .type(Symbol.builder().name("str").build()) + .type(Symbol.builder().name("str | None").build()) .documentation(" The AWS region to connect to. The configured region is used to " + "determine the service endpoint.") + .nullable(false) + .useDescriptor(true) + .validator(Symbol.builder() + .name("validate_region") + .namespace("smithy_aws_core.config.validators", ".") + .addDependency(AwsPythonDependency.SMITHY_AWS_CORE) + .build()) + .build(); + + public static final ConfigProperty RETRY_STRATEGY = ConfigProperty.builder() + .name("retry_strategy") + .type(Symbol.builder() + .name("RetryStrategy | RetryStrategyOptions | None") + .addReference(Symbol.builder() + .name("RetryStrategy") + .namespace("smithy_core.interfaces.retries", ".") + .addDependency(software.amazon.smithy.python.codegen.SmithyPythonDependency.SMITHY_CORE) + .build()) + .addReference(Symbol.builder() + .name("RetryStrategyOptions") + .namespace("smithy_core.retries", ".") + .addDependency(software.amazon.smithy.python.codegen.SmithyPythonDependency.SMITHY_CORE) + .build()) + .build()) + .documentation( + "The retry strategy or options for configuring retry behavior. Can be either a configured RetryStrategy or RetryStrategyOptions to create one.") + .nullable(false) + .useDescriptor(true) + .validator(Symbol.builder() + .name("validate_retry_strategy") + .namespace("smithy_aws_core.config.validators", ".") + .addDependency(AwsPythonDependency.SMITHY_AWS_CORE) + .build()) + .customResolver(Symbol.builder() + .name("resolve_retry_strategy") + .namespace("smithy_aws_core.config.custom_resolvers", ".") + .addDependency(AwsPythonDependency.SMITHY_AWS_CORE) + .build()) + .defaultValue("RetryStrategyOptions(retry_mode=\"standard\")") .build(); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/ConfigProperty.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/ConfigProperty.java index 7a8b1d674..bddbfe6d5 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/ConfigProperty.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/ConfigProperty.java @@ -23,6 +23,10 @@ public final class ConfigProperty implements ToSmithyBuilder { private final boolean nullable; private final String documentation; private final Consumer initialize; + private final Symbol validator; + private final Symbol customResolver; + private final boolean useDescriptor; + private final String defaultValue; /** * Constructor. @@ -33,6 +37,10 @@ private ConfigProperty(Builder builder) { this.nullable = builder.nullable; this.documentation = Objects.requireNonNull(builder.documentation); this.initialize = Objects.requireNonNull(builder.initialize); + this.validator = builder.validator; + this.customResolver = builder.customResolver; + this.useDescriptor = builder.useDescriptor; + this.defaultValue = builder.defaultValue; } /** @@ -63,6 +71,34 @@ public String documentation() { return documentation; } + /** + * @return Returns the validator symbol for this property, if any. + */ + public java.util.Optional validator() { + return java.util.Optional.ofNullable(validator); + } + + /** + * @return Returns the custom resolver symbol for this property, if any. + */ + public java.util.Optional customResolver() { + return java.util.Optional.ofNullable(customResolver); + } + + /** + * @return Returns whether this property uses the ConfigProperty descriptor. + */ + public boolean useDescriptor() { + return useDescriptor; + } + + /** + * @return Returns the default value for this property, if any. + */ + public java.util.Optional defaultValue() { + return java.util.Optional.ofNullable(defaultValue); + } + /** * Initializes the config field on the config object. * @@ -94,7 +130,11 @@ public SmithyBuilder toBuilder() { .type(type) .nullable(nullable) .documentation(documentation) - .initialize(initialize); + .initialize(initialize) + .validator(validator) + .customResolver(customResolver) + .useDescriptor(useDescriptor) + .defaultValue(defaultValue); } /** @@ -107,6 +147,11 @@ public static final class Builder implements SmithyBuilder { private String documentation; private Consumer initialize = writer -> writer.write("self.$1L = $1L", name); + private Symbol validator; + private Symbol customResolver; + private boolean useDescriptor = false; + private String defaultValue; + @Override public ConfigProperty build() { return new ConfigProperty(this); @@ -182,5 +227,49 @@ public Builder initialize(Consumer initialize) { this.initialize = initialize; return this; } + + /** + * Sets the validator symbol for the config property. + * + * @param validator The validator function symbol. + * @return Returns the builder. + */ + public Builder validator(Symbol validator) { + this.validator = validator; + return this; + } + + /** + * Sets the custom resolver symbol for the config property. + * + * @param customResolver The custom resolver function symbol. + * @return Returns the builder. + */ + public Builder customResolver(Symbol customResolver) { + this.customResolver = customResolver; + return this; + } + + /** + * Sets whether the config property uses the ConfigProperty descriptor. + * + * @param useDescriptor Whether to use the descriptor pattern. + * @return Returns the builder. + */ + public Builder useDescriptor(boolean useDescriptor) { + this.useDescriptor = useDescriptor; + return this; + } + + /** + * Sets the default value for the config property. + * + * @param defaultValue The default value as a Python expression string. + * @return Returns the builder. + */ + public Builder defaultValue(String defaultValue) { + this.defaultValue = defaultValue; + return this; + } } } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java index de03d42d0..e073ead8f 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java @@ -52,24 +52,6 @@ public final class ConfigGenerator implements Runnable { .nullable(false) .initialize(writer -> writer.write("self.interceptors = interceptors or []")) .build(), - ConfigProperty.builder() - .name("retry_strategy") - .type(Symbol.builder() - .name("RetryStrategy | RetryStrategyOptions") - .addReference(Symbol.builder() - .name("RetryStrategy") - .namespace("smithy_core.interfaces.retries", ".") - .addDependency(SmithyPythonDependency.SMITHY_CORE) - .build()) - .addReference(Symbol.builder() - .name("RetryStrategyOptions") - .namespace("smithy_core.retries", ".") - .addDependency(SmithyPythonDependency.SMITHY_CORE) - .build()) - .build()) - .documentation( - "The retry strategy or options for configuring retry behavior. Can be either a configured RetryStrategy or RetryStrategyOptions to create one.") - .build(), ConfigProperty.builder() .name("endpoint_uri") .type(Symbol.builder() @@ -98,6 +80,23 @@ public final class ConfigGenerator implements Runnable { writer.write("self.endpoint_resolver = endpoint_resolver or StaticEndpointResolver()"); writer.popState(); }) + .build(), + ConfigProperty.builder() + .name("retry_strategy") + .type(Symbol.builder() + .name("RetryStrategy | RetryStrategyOptions | None") + .addReference(Symbol.builder() + .name("RetryStrategy") + .namespace("smithy_core.interfaces.retries", ".") + .addDependency(SmithyPythonDependency.SMITHY_CORE) + .build()) + .addReference(Symbol.builder() + .name("RetryStrategyOptions") + .namespace("smithy_core.retries", ".") + .addDependency(SmithyPythonDependency.SMITHY_CORE) + .build()) + .build()) + .documentation("The retry strategy or options for configuring retry behavior. Can be either a configured RetryStrategy or RetryStrategyOptions to create one.") .build()); // This list contains any properties that must be added to any http-based @@ -310,8 +309,21 @@ private void writeInterceptorsType(PythonWriter writer) { private void generateConfig(GenerationContext context, PythonWriter writer) { var configSymbol = CodegenUtils.getConfigSymbol(context.settings()); - // Initialize a set of config properties with our base properties. + // Initialize a set of config properties. var properties = new TreeSet<>(Comparator.comparing(ConfigProperty::name)); + + var model = context.model(); + var service = context.settings().service(model); + + // Add plugin properties first so they can override base properties with same name. + for (PythonIntegration integration : context.integrations()) { + for (RuntimeClientPlugin plugin : integration.getClientPlugins(context)) { + if (plugin.matchesService(model, service)) { + properties.addAll(plugin.getConfigProperties()); + } + } + } + properties.addAll(BASE_PROPERTIES); properties.addAll(getProtocolProperties(context)); @@ -322,19 +334,43 @@ private void generateConfig(GenerationContext context, PythonWriter writer) { writer.onSection(new AddAuthHelper()); } - var model = context.model(); - var service = context.settings().service(model); + writer.onSection(new AddGetSourceHelper()); - // Add any relevant config properties from plugins. - for (PythonIntegration integration : context.integrations()) { - for (RuntimeClientPlugin plugin : integration.getClientPlugins(context)) { - if (plugin.matchesService(model, service)) { - properties.addAll(plugin.getConfigProperties()); + var finalProperties = List.copyOf(properties); + + // Check if any properties use descriptors + boolean hasDescriptors = finalProperties.stream().anyMatch(ConfigProperty::useDescriptor); + + // Only add config resolution imports if there are descriptor properties + if (hasDescriptors) { + writer.addDependency(SmithyPythonDependency.SMITHY_CORE); + writer.addImport("smithy_core.config.property", "ConfigProperty"); + writer.addImport("smithy_core.config.resolver", "ConfigResolver"); + writer.addImport("smithy_aws_core.config.sources", "EnvironmentSource"); + + // Add validator and resolver imports for properties that use descriptors + for (ConfigProperty property : finalProperties) { + if (property.useDescriptor()) { + if (property.validator().isPresent()) { + var validatorSymbol = property.validator().get(); + writer.addImport(validatorSymbol.getNamespace(), validatorSymbol.getName()); + } + if (property.customResolver().isPresent()) { + var resolverSymbol = property.customResolver().get(); + writer.addImport(resolverSymbol.getNamespace(), resolverSymbol.getName()); + } + // Add imports for types referenced in default values + if (property.defaultValue().isPresent()) { + var defaultValue = property.defaultValue().get(); + if (defaultValue.contains("RetryStrategyOptions")) { + writer.addDependency(SmithyPythonDependency.SMITHY_CORE); + writer.addImport("smithy_core.retries", "RetryStrategyOptions"); + } + } } } } - var finalProperties = List.copyOf(properties); final String serviceId = context.settings() .service(context.model()) .getTrait(ServiceTrait.class) @@ -349,6 +385,8 @@ class $L: ${C|} + ${C|} + def __init__( self, *, @@ -359,31 +397,134 @@ def __init__( configSymbol.getName(), serviceId, writer.consumer(w -> writePropertyDeclarations(w, finalProperties)), + writer.consumer(w -> writeDescriptorDeclarations(w, finalProperties, context)), writer.consumer(w -> writeInitParams(w, finalProperties)), writer.consumer(w -> initializeProperties(w, finalProperties))); writer.popState(); } + // Write descriptor declarations for properties using ConfigProperty descriptor + private void writeDescriptorDeclarations(PythonWriter writer, Collection properties, GenerationContext context) { + boolean hasDescriptors = properties.stream().anyMatch(ConfigProperty::useDescriptor); + + if (!hasDescriptors) { + return; + } + + writer.write("# Config properties using descriptors"); + writer.write("_descriptors = {"); + writer.indent(); + + for (ConfigProperty property : properties) { + if (property.useDescriptor()) { + writer.writeInline("'$L': ConfigProperty('$L'", + property.name(), + property.name()); + + if (property.validator().isPresent()) { + writer.writeInline(", validator=$L", property.validator().get().getName()); + } + + if (property.customResolver().isPresent()) { + writer.writeInline(", resolver_func=$L", property.customResolver().get().getName()); + } + + if (property.defaultValue().isPresent()) { + writer.writeInline(", default_value=$L", property.defaultValue().get()); + } + + writer.write("),"); + } + } + + writer.dedent(); + writer.write("}"); + writer.write(""); + + for (ConfigProperty property : properties) { + if (property.useDescriptor()) { + var typeHint = property.isNullable() + ? "$T | None" + : "$T"; + writer.write("$L: " + typeHint + " = _descriptors['$L'] # type: ignore[assignment]", + property.name(), + property.type(), + property.name()); + + if (!property.documentation().isEmpty()) { + writer.writeDocs(property.documentation(), context); + } + writer.write(""); + } + } + writer.write(""); + } + private void writePropertyDeclarations(PythonWriter writer, Collection properties) { for (ConfigProperty property : properties) { - var formatString = property.isNullable() - ? "$L: $T | None" - : "$L: $T"; + // Skip descriptor properties - they're declared above + if (property.useDescriptor()) { + continue; + } + + String typeName = property.type().getName(); + String formatString; + if (property.isNullable() && !typeName.endsWith("| None")) { + formatString = "$L: $T | None"; + } else { + formatString = "$L: $T"; + } writer.write(formatString, property.name(), property.type()); writer.writeDocs(property.documentation(), context); writer.write(""); } + + // Add _resolver declaration only if there are descriptor properties + boolean hasDescriptors = properties.stream().anyMatch(ConfigProperty::useDescriptor); + if (hasDescriptors) { + writer.write("_resolver: ConfigResolver"); + writer.write(""); + } } private void writeInitParams(PythonWriter writer, Collection properties) { for (ConfigProperty property : properties) { - writer.write("$L: $T | None = None,", property.name(), property.type()); + String typeName = property.type().getName(); + if (typeName.endsWith("| None")) { + writer.write("$L: $T = None,", property.name(), property.type()); + } else { + writer.write("$L: $T | None = None,", property.name(), property.type()); + } } } private void initializeProperties(PythonWriter writer, Collection properties) { + var descriptorProperties = properties.stream() + .filter(ConfigProperty::useDescriptor) + .toList(); + + if (!descriptorProperties.isEmpty()) { + writer.write("# Set instance values for descriptor properties"); + writer.write("self._resolver = ConfigResolver(sources=[EnvironmentSource()])"); + writer.write(""); + + writer.write("# Only set if provided (not None) to allow resolution from sources"); + writer.write("for key in self.__class__._descriptors.keys():"); + writer.indent(); + writer.write("value = locals().get(key)"); + writer.write("if value is not None:"); + writer.indent(); + writer.write("setattr(self, key, value)"); + writer.dedent(); + writer.dedent(); + writer.write(""); + } + + // Finally, initialize non-descriptor properties normally for (ConfigProperty property : properties) { - property.initialize(writer); + if (!property.useDescriptor()) { + property.initialize(writer); + } } } @@ -418,4 +559,43 @@ def set_auth_scheme(self, scheme: AuthScheme[Any, Any, Any, Any]) -> None: """); } } + + private static final class AddGetSourceHelper implements CodeInterceptor { + @Override + public Class sectionType() { + return ConfigSection.class; + } + + @Override + public void write(PythonWriter writer, String previousText, ConfigSection section) { + // Check if there are any descriptor properties + boolean hasDescriptors = section.properties() + .stream() + .anyMatch(ConfigProperty::useDescriptor); + + if (!hasDescriptors) { + // No descriptor properties, just write previous text + writer.write(previousText); + return; + } + + writer.write(previousText); + + writer.write(""" + + def get_source(self, key: str) -> str | None: + \"""Get the source that provided a configuration value. + + Args: + key: The configuration key (e.g., 'region', 'retry_strategy') + + Returns: + The source name ('instance', 'environment', etc.), + or None if the key hasn't been resolved yet. + \""" + cached = self.__dict__.get(f'_cache_{key}') + return cached[1] if cached else None + """); + } + } } diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py index 675570695..3aa87cc15 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py @@ -40,8 +40,8 @@ def resolve_retry_strategy( max_attempts = validate_max_attempts(max_attempts, attempts_source) options = RetryStrategyOptions( - retry_mode=retry_mode, # type: ignore - max_attempts=max_attempts, # Can be None because strategy will use its default + retry_mode=retry_mode or "standard", # type: ignore + max_attempts=max_attempts, ) # Construct mixed source string showing where each component came from diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py index 65f9a536f..eef62275b 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py @@ -23,7 +23,7 @@ def __init__(self, key: str, value: Any, reason: str, source: str | None = None) super().__init__(msg) -def validate_region(region: str, source: str | None = None) -> str: +def validate_region(region: str | None, source: str | None = None) -> str: """Validate region name format. :param region: The value to validate @@ -33,6 +33,14 @@ def validate_region(region: str, source: str | None = None) -> str: :raises ConfigValidationError: If the value format is invalid """ + if region is None: + raise ConfigValidationError( + "region", + region, + "region not found. It is required and must be explicitly set.", + source, + ) + pattern = r"^(?![0-9]+$)(?!-)[a-zA-Z0-9-]{1,63}(? str: def validate_retry_mode(retry_mode: str, source: str | None = None) -> str: """Validate retry mode. - Valid values: 'standard', 'simple' + Valid values: 'standard' :param retry_mode: The retry mode value to validate :param source: The source that provided this value @@ -57,8 +65,14 @@ def validate_retry_mode(retry_mode: str, source: str | None = None) -> str: :raises: ConfigValidationError: If the retry mode is invalid """ - - valid_modes = get_args(RetryStrategyType) + # NOTE: RetryStrategyType supports 'simple' as a direct config value, but the valid + # string modes here are restricted to align with the standard AWS retry modes: + # 'standard' and 'adaptive'. 'legacy' is intentionally excluded as it is not + # recommended. A simple retry strategy can still be provided directly via the config. + all_modes = list(get_args(RetryStrategyType)) + if "simple" in all_modes: + all_modes.remove("simple") + valid_modes = tuple(all_modes) if retry_mode not in valid_modes: raise ConfigValidationError( diff --git a/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py index c9e7d23e9..623614649 100644 --- a/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py +++ b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py @@ -73,6 +73,8 @@ def test_returns_strategy_when_only_retry_mode_set(self) -> None: assert isinstance(result, RetryStrategyOptions) assert result.retry_mode == "standard" + # None for max_attempts means the RetryStrategy will use its + # own default max_attempts value for the set retry_mode assert result.max_attempts is None assert source_name == "retry_mode=environment, max_attempts=default" @@ -84,6 +86,7 @@ def test_returns_strategy_when_only_max_attempts_set(self) -> None: assert isinstance(result, RetryStrategyOptions) assert result.max_attempts == 5 + assert result.retry_mode == "standard" assert source_name == "retry_mode=default, max_attempts=environment" def test_returns_none_when_both_values_missing(self) -> None: diff --git a/packages/smithy-aws-core/tests/unit/config/test_validators.py b/packages/smithy-aws-core/tests/unit/config/test_validators.py index 2ce809c90..546af5365 100644 --- a/packages/smithy-aws-core/tests/unit/config/test_validators.py +++ b/packages/smithy-aws-core/tests/unit/config/test_validators.py @@ -21,7 +21,7 @@ def test_validate_region_rejects_invalid_values(self, invalid: str) -> None: with pytest.raises(ConfigValidationError): validate_region(invalid) - @pytest.mark.parametrize("mode", ["standard", "simple"]) + @pytest.mark.parametrize("mode", ["standard"]) def test_validate_retry_mode_accepts_valid_values(self, mode: str) -> None: assert validate_retry_mode(mode) == mode @@ -47,5 +47,5 @@ def test_invalid_retry_mode_error_message(self) -> None: validate_retry_mode("random_mode") assert ( "Invalid value for 'retry_mode': 'random_mode'. retry_mode must be one " - "of ('simple', 'standard'), got random_mode" in str(exc_info.value) + "of ('standard',), got random_mode" in str(exc_info.value) ) From 6c4b348e0094f4b631a55a2c7be8f73ad2286bd6 Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Thu, 12 Mar 2026 23:14:34 -0400 Subject: [PATCH 5/6] Replace raw source strings with typed SourceInfo dataclasses (#652) * Replace raw source strings with typed SourceInfo dataclasses --- .../codegen/generators/ConfigGenerator.java | 12 ++- .../config/custom_resolvers.py | 16 ++- .../src/smithy_aws_core}/config/property.py | 16 ++- .../src/smithy_aws_core}/config/resolver.py | 6 +- .../src/smithy_aws_core/config/source_info.py | 48 +++++++++ .../src/smithy_aws_core/config/sources.py | 4 +- .../src/smithy_aws_core/config/validators.py | 16 ++- .../tests/unit/config/test_custom_resolver.py | 19 +++- .../tests/unit/config/test_property.py | 100 +++++++++++++----- .../tests/unit/config/test_resolver.py | 23 ++-- .../src/smithy_core/config/__init__.py | 8 -- .../smithy-core/tests/unit/config/__init__.py | 2 - 12 files changed, 198 insertions(+), 72 deletions(-) rename packages/{smithy-core/src/smithy_core => smithy-aws-core/src/smithy_aws_core}/config/property.py (87%) rename packages/{smithy-core/src/smithy_core => smithy-aws-core/src/smithy_aws_core}/config/resolver.py (86%) create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/config/source_info.py rename packages/{smithy-core => smithy-aws-core}/tests/unit/config/test_property.py (69%) rename packages/{smithy-core => smithy-aws-core}/tests/unit/config/test_resolver.py (79%) delete mode 100644 packages/smithy-core/src/smithy_core/config/__init__.py delete mode 100644 packages/smithy-core/tests/unit/config/__init__.py diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java index e073ead8f..92aa857e5 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java @@ -343,9 +343,9 @@ private void generateConfig(GenerationContext context, PythonWriter writer) { // Only add config resolution imports if there are descriptor properties if (hasDescriptors) { - writer.addDependency(SmithyPythonDependency.SMITHY_CORE); - writer.addImport("smithy_core.config.property", "ConfigProperty"); - writer.addImport("smithy_core.config.resolver", "ConfigResolver"); + writer.addDependency(SmithyPythonDependency.SMITHY_AWS_CORE); + writer.addImport("smithy_aws_core.config.property", "ConfigProperty"); + writer.addImport("smithy_aws_core.config.resolver", "ConfigResolver"); writer.addImport("smithy_aws_core.config.sources", "EnvironmentSource"); // Add validator and resolver imports for properties that use descriptors @@ -580,17 +580,19 @@ public void write(PythonWriter writer, String previousText, ConfigSection sectio } writer.write(previousText); + writer.addImport("smithy_aws_core.config.source_info", "SourceInfo"); writer.write(""" - def get_source(self, key: str) -> str | None: + def get_source(self, key: str) -> SourceInfo | None: \"""Get the source that provided a configuration value. Args: key: The configuration key (e.g., 'region', 'retry_strategy') Returns: - The source name ('instance', 'environment', etc.), + The source info (SimpleSource('source_name') or + ComplexSource({"retry_mode": "source1", "max_attempts": "source2"})), or None if the key hasn't been resolved yet. \""" cached = self.__dict__.get(f'_cache_{key}') diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py index 3aa87cc15..5aa7de408 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/custom_resolvers.py @@ -1,15 +1,16 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from smithy_core.config.resolver import ConfigResolver from smithy_core.retries import RetryStrategyOptions +from smithy_aws_core.config.resolver import ConfigResolver +from smithy_aws_core.config.source_info import ComplexSource, SourceName from smithy_aws_core.config.validators import validate_max_attempts, validate_retry_mode def resolve_retry_strategy( resolver: ConfigResolver, -) -> tuple[RetryStrategyOptions | None, str | None]: +) -> tuple[RetryStrategyOptions | None, ComplexSource | None]: """Resolve retry strategy from multiple config keys. Resolves both retry_mode and max_attempts from sources and constructs @@ -23,7 +24,7 @@ def resolve_retry_strategy( are resolved. Returns (None, None) if both values are missing. For mixed sources, the source name includes both component sources: - "retry_mode=environment, max_attempts=config_file" + {"retry_mode": "environment", "max_attempts": "default"} """ retry_mode, mode_source = resolver.get("retry_mode") @@ -45,6 +46,13 @@ def resolve_retry_strategy( ) # Construct mixed source string showing where each component came from - source = f"retry_mode={mode_source or 'default'}, max_attempts={attempts_source or 'default'}" + source = ComplexSource( + { + "retry_mode": mode_source.name if mode_source else SourceName.DEFAULT, + "max_attempts": attempts_source.name + if attempts_source + else SourceName.DEFAULT, + } + ) return (options, source) diff --git a/packages/smithy-core/src/smithy_core/config/property.py b/packages/smithy-aws-core/src/smithy_aws_core/config/property.py similarity index 87% rename from packages/smithy-core/src/smithy_core/config/property.py rename to packages/smithy-aws-core/src/smithy_aws_core/config/property.py index 71a3f413c..56e136981 100644 --- a/packages/smithy-core/src/smithy_core/config/property.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/property.py @@ -3,7 +3,8 @@ from collections.abc import Callable from typing import Any -from smithy_core.config.resolver import ConfigResolver +from smithy_aws_core.config.resolver import ConfigResolver +from smithy_aws_core.config.source_info import SimpleSource, SourceInfo, SourceName class ConfigProperty: @@ -27,8 +28,9 @@ def __init__(self): def __init__( self, key: str, - validator: Callable[[Any, str | None], Any] | None = None, - resolver_func: Callable[[ConfigResolver], tuple[Any, str | None]] | None = None, + validator: Callable[[Any, SourceInfo | None], Any] | None = None, + resolver_func: Callable[[ConfigResolver], tuple[Any, SourceInfo | None]] + | None = None, default_value: Any = None, ): """Initialize config property descriptor. @@ -78,7 +80,7 @@ def __get__(self, obj: Any, objtype: type | None = None) -> Any: if value is None: value = self.default_value - source = "default" + source = SimpleSource(SourceName.DEFAULT) if self.validator: value = self.validator(value, source) @@ -99,7 +101,11 @@ def __set__(self, obj: Any, value: Any) -> None: # Determine source based on when the value was set # If cache already exists, it means it was not set during initialization # In that case source will be set to in-code - source = "in-code" if hasattr(obj, self.cache_attr) else "instance" + source = ( + SimpleSource(SourceName.IN_CODE) + if hasattr(obj, self.cache_attr) + else SimpleSource(SourceName.INSTANCE) + ) if self.validator: value = self.validator(value, source) diff --git a/packages/smithy-core/src/smithy_core/config/resolver.py b/packages/smithy-aws-core/src/smithy_aws_core/config/resolver.py similarity index 86% rename from packages/smithy-core/src/smithy_core/config/resolver.py rename to packages/smithy-aws-core/src/smithy_aws_core/config/resolver.py index 7a82ca2b6..a17a36b93 100644 --- a/packages/smithy-core/src/smithy_core/config/resolver.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/resolver.py @@ -5,6 +5,8 @@ from smithy_core.interfaces.config import ConfigSource +from smithy_aws_core.config.source_info import SimpleSource + class ConfigResolver: """Resolves configuration values from multiple sources. @@ -21,7 +23,7 @@ def __init__(self, sources: Sequence[ConfigSource]) -> None: """ self._sources = sources - def get(self, key: str) -> tuple[Any, str | None]: + def get(self, key: str) -> tuple[Any, SimpleSource | None]: """Resolve a configuration value from sources by iterating through them in precedence order. :param key: The configuration key to resolve (e.g., 'retry_mode') @@ -32,5 +34,5 @@ def get(self, key: str) -> tuple[Any, str | None]: for source in self._sources: value = source.get(key) if value is not None: - return (value, source.name) + return (value, SimpleSource(source.name)) return (None, None) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/source_info.py b/packages/smithy-aws-core/src/smithy_aws_core/config/source_info.py new file mode 100644 index 000000000..64e5d0cc2 --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/source_info.py @@ -0,0 +1,48 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +from dataclasses import dataclass +from enum import StrEnum + + +class SourceName(StrEnum): + """Known source names for config value provenance tracking.""" + + INSTANCE = "instance" # value provided via Config constructor + + IN_CODE = "in-code" # value set via setter after Config construction + + ENVIRONMENT = "environment" # value resolved from environment variable + + DEFAULT = "default" # value fall back to default + + +@dataclass(frozen=True) +class SimpleSource: + """Source info for a config value resolved from a single source. + + Examples: region from environment, max_attempts from config file. + """ + + # TODO: Currently only environment variable is implemented as a config + # source. Tests use raw strings (e.g., "environment", "config_file") as + # source names to simulate multi-source scenarios. Once additional + # config sources are implemented, update the `name` parameter type + # from `str` to `SourceName` and replace raw strings in tests with + # the corresponding enum values. + name: str + + +@dataclass(frozen=True) +class ComplexSource: + """Source info for a config value resolved from multiple sources. + + Used when a config property is composed of multiple sources. + Example: retry_strategy is composed of retry_mode and max_attempts and they both + could be from different sources: {"retry_mode": "environment", "max_attempts": "config_file"} + """ + + components: dict[str, str] + + +SourceInfo = SimpleSource | ComplexSource diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py b/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py index f5e5195de..26c5f3550 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/sources.py @@ -2,6 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 import os +from smithy_aws_core.config.source_info import SourceName + class EnvironmentSource: """Configuration from environment variables.""" @@ -16,7 +18,7 @@ def __init__(self, prefix: str = "AWS_"): @property def name(self) -> str: """Returns the source name.""" - return "environment" + return SourceName.ENVIRONMENT def get(self, key: str) -> str | None: """Returns a configuration value from environment variables. diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py index eef62275b..41b670e02 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py @@ -7,11 +7,15 @@ from smithy_core.interfaces.retries import RetryStrategy from smithy_core.retries import RetryStrategyOptions, RetryStrategyType +from smithy_aws_core.config.source_info import SourceInfo + class ConfigValidationError(ValueError): """Raised when a configuration value fails validation.""" - def __init__(self, key: str, value: Any, reason: str, source: str | None = None): + def __init__( + self, key: str, value: Any, reason: str, source: SourceInfo | None = None + ): self.key = key self.value = value self.reason = reason @@ -23,7 +27,7 @@ def __init__(self, key: str, value: Any, reason: str, source: str | None = None) super().__init__(msg) -def validate_region(region: str | None, source: str | None = None) -> str: +def validate_region(region: str | None, source: SourceInfo | None = None) -> str: """Validate region name format. :param region: The value to validate @@ -53,7 +57,7 @@ def validate_region(region: str | None, source: str | None = None) -> str: return region -def validate_retry_mode(retry_mode: str, source: str | None = None) -> str: +def validate_retry_mode(retry_mode: str, source: SourceInfo | None = None) -> str: """Validate retry mode. Valid values: 'standard' @@ -85,7 +89,9 @@ def validate_retry_mode(retry_mode: str, source: str | None = None) -> str: return retry_mode -def validate_max_attempts(max_attempts: str | int, source: str | None = None) -> int: +def validate_max_attempts( + max_attempts: str | int, source: SourceInfo | None = None +) -> int: """Validate and convert max_attempts to integer. :param max_attempts: The max attempts value (string or int) @@ -117,7 +123,7 @@ def validate_max_attempts(max_attempts: str | int, source: str | None = None) -> def validate_retry_strategy( - value: Any, source: str | None = None + value: Any, source: SourceInfo | None = None ) -> RetryStrategy | RetryStrategyOptions: """Validate retry strategy configuration. diff --git a/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py index 623614649..37657598a 100644 --- a/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py +++ b/packages/smithy-aws-core/tests/unit/config/test_custom_resolver.py @@ -4,7 +4,8 @@ from typing import Any from smithy_aws_core.config.custom_resolvers import resolve_retry_strategy -from smithy_core.config.resolver import ConfigResolver +from smithy_aws_core.config.resolver import ConfigResolver +from smithy_aws_core.config.source_info import ComplexSource from smithy_core.retries import RetryStrategyOptions @@ -39,7 +40,9 @@ def test_resolves_from_both_values(self) -> None: assert isinstance(result, RetryStrategyOptions) assert result.retry_mode == "standard" assert result.max_attempts == 3 - assert source_name == "retry_mode=environment, max_attempts=environment" + assert source_name == ComplexSource( + {"retry_mode": "environment", "max_attempts": "environment"} + ) def test_tracks_different_sources_for_each_component(self) -> None: source1 = StubSource("environment", {"retry_mode": "standard"}) @@ -51,7 +54,9 @@ def test_tracks_different_sources_for_each_component(self) -> None: assert isinstance(result, RetryStrategyOptions) assert result.retry_mode == "standard" assert result.max_attempts == 5 - assert source_name == "retry_mode=environment, max_attempts=config_file" + assert source_name == ComplexSource( + {"retry_mode": "environment", "max_attempts": "config_file"} + ) def test_converts_max_attempts_string_to_int(self) -> None: source = StubSource( @@ -76,7 +81,9 @@ def test_returns_strategy_when_only_retry_mode_set(self) -> None: # None for max_attempts means the RetryStrategy will use its # own default max_attempts value for the set retry_mode assert result.max_attempts is None - assert source_name == "retry_mode=environment, max_attempts=default" + assert source_name == ComplexSource( + {"retry_mode": "environment", "max_attempts": "default"} + ) def test_returns_strategy_when_only_max_attempts_set(self) -> None: source = StubSource("environment", {"max_attempts": "5"}) @@ -87,7 +94,9 @@ def test_returns_strategy_when_only_max_attempts_set(self) -> None: assert isinstance(result, RetryStrategyOptions) assert result.max_attempts == 5 assert result.retry_mode == "standard" - assert source_name == "retry_mode=default, max_attempts=environment" + assert source_name == ComplexSource( + {"retry_mode": "default", "max_attempts": "environment"} + ) def test_returns_none_when_both_values_missing(self) -> None: source = StubSource("environment", {}) diff --git a/packages/smithy-core/tests/unit/config/test_property.py b/packages/smithy-aws-core/tests/unit/config/test_property.py similarity index 69% rename from packages/smithy-core/tests/unit/config/test_property.py rename to packages/smithy-aws-core/tests/unit/config/test_property.py index 88ae990a6..42ea31291 100644 --- a/packages/smithy-core/tests/unit/config/test_property.py +++ b/packages/smithy-aws-core/tests/unit/config/test_property.py @@ -5,8 +5,10 @@ from typing import Any, NoReturn import pytest -from smithy_core.config.property import ConfigProperty -from smithy_core.config.resolver import ConfigResolver +from smithy_aws_core.config.property import ConfigProperty +from smithy_aws_core.config.resolver import ConfigResolver +from smithy_aws_core.config.source_info import SimpleSource, SourceInfo +from smithy_core.retries import RetryStrategyOptions class StubSource: @@ -60,7 +62,10 @@ def test_caches_resolved_value(self) -> None: def test_uses_default_value_when_unresolved(self) -> None: class ConfigWithDefault: - region = ConfigProperty("region", default_value="us-east-1") + retry_strategy = ConfigProperty( + "retry_strategy", + default_value=RetryStrategyOptions(retry_mode="standard"), + ) def __init__(self, resolver: ConfigResolver) -> None: self._resolver = resolver @@ -69,10 +74,14 @@ def __init__(self, resolver: ConfigResolver) -> None: resolver = ConfigResolver(sources=[source]) config = ConfigWithDefault(resolver) - result = config.region + result = config.retry_strategy - assert result == "us-east-1" - assert getattr(config, "_cache_region") == ("us-east-1", "default") + assert result.retry_mode == "standard" + assert result.max_attempts is None + assert getattr(config, "_cache_retry_strategy") == ( + RetryStrategyOptions(retry_mode="standard"), + SimpleSource("default"), + ) def test_different_properties_resolve_independently(self) -> None: source = StubSource( @@ -92,12 +101,13 @@ class TestConfigPropertyValidation: """Test suite for ConfigProperty validation behavior.""" def _create_config_with_validator( - self, validator: Callable[[Any, str | None], Any] + self, validator: Callable[[Any, SourceInfo | None], Any] ) -> type[Any]: """Helper to create a config class with a specific validator.""" class ConfigWithValidator: region = ConfigProperty("region", validator=validator) + retry_strategy = ConfigProperty("retry_strategy", validator=validator) def __init__(self, resolver: ConfigResolver) -> None: self._resolver = resolver @@ -105,9 +115,9 @@ def __init__(self, resolver: ConfigResolver) -> None: return ConfigWithValidator def test_calls_validator_on_resolution(self) -> None: - call_log: list[tuple[Any, str | None]] = [] + call_log: list[tuple[Any, SourceInfo | None]] = [] - def mock_validator(value: Any, source: str | None) -> Any: + def mock_validator(value: Any, source: SourceInfo | None) -> Any: call_log.append((value, source)) return value @@ -120,10 +130,10 @@ def mock_validator(value: Any, source: str | None) -> Any: assert result == "us-west-2" assert len(call_log) == 1 - assert call_log[0] == ("us-west-2", "environment") + assert call_log[0] == ("us-west-2", SimpleSource("environment")) def test_validator_exception_propagates(self) -> None: - def failing_validator(value: Any, source: str | None) -> NoReturn: + def failing_validator(value: Any, source: SourceInfo | None) -> NoReturn: raise ValueError("Invalid value") ConfigWithValidator = self._create_config_with_validator(failing_validator) @@ -134,10 +144,40 @@ def failing_validator(value: Any, source: str | None) -> NoReturn: with pytest.raises(ValueError, match="Invalid value"): config.region + def test_complex_resolver_falls_back_to_default(self) -> None: + def mock_resolver(resolver: ConfigResolver) -> tuple[None, None]: + # Simulates resolve_retry_strategy returning (None, None) when no sources have values + return (None, None) + + class ConfigWithComplexResolver: + retry_strategy = ConfigProperty( + "retry_strategy", + resolver_func=mock_resolver, + default_value=RetryStrategyOptions( + retry_mode="standard", max_attempts=3 + ), + ) + + def __init__(self, resolver: ConfigResolver) -> None: + self._resolver = resolver + + source = StubSource("environment", {}) + resolver = ConfigResolver(sources=[source]) + config = ConfigWithComplexResolver(resolver) + + result = config.retry_strategy + cached = getattr(config, "_cache_retry_strategy", None) + source_info = cached[1] if cached else None + + assert isinstance(result, RetryStrategyOptions) + assert result.retry_mode == "standard" + assert result.max_attempts == 3 + assert source_info == SimpleSource("default") + def test_validator_not_called_on_cached_access(self) -> None: call_count = 0 - def counting_validator(value: Any, source: str | None) -> Any: + def counting_validator(value: Any, source: SourceInfo | None) -> Any: nonlocal call_count call_count += 1 return value @@ -167,7 +207,10 @@ def test_set_value_marks_source_as_instance(self) -> None: config.region = "eu-west-1" # Check the cached tuple - assert getattr(config, "_cache_region") == ("eu-west-1", "instance") + assert getattr(config, "_cache_region") == ( + "eu-west-1", + SimpleSource("instance"), + ) def test_value_set_after_resolution_marks_source_as_in_code(self) -> None: source = StubSource("environment", {"region": "us-west-2"}) @@ -184,12 +227,15 @@ def test_value_set_after_resolution_marks_source_as_in_code(self) -> None: assert config.region == "eu-west-1" # Verify source is marked as 'in-code' # Any config value modified after initialization will have 'in-code' for source - assert getattr(config, "_cache_region") == ("eu-west-1", "in-code") + assert getattr(config, "_cache_region") == ( + "eu-west-1", + SimpleSource("in-code"), + ) def test_validator_is_called_when_setting_values(self) -> None: - call_log: list[tuple[Any, str | None]] = [] + call_log: list[tuple[Any, SourceInfo | None]] = [] - def mock_validator(value: Any, source: str | None) -> Any: + def mock_validator(value: Any, source: SourceInfo | None) -> Any: call_log.append((value, source)) return value @@ -207,10 +253,10 @@ def __init__(self, resolver: ConfigResolver) -> None: assert config.region == "us-west-2" assert len(call_log) == 1 - assert call_log[0] == ("us-west-2", "instance") + assert call_log[0] == ("us-west-2", SimpleSource("instance")) def test_validator_throws_exception_when_setting_invalid_value(self) -> None: - def mock_failing_validation(value: Any, source: str | None) -> NoReturn: + def mock_failing_validation(value: Any, source: SourceInfo | None) -> NoReturn: raise ValueError("Invalid value") class ConfigWithValidator: @@ -251,18 +297,20 @@ def test_cache_stores_value_and_source_as_tuple(self) -> None: config.region cached: Any = getattr(config, "_cache_region") - assert cached == ("us-west-2", "environment") + assert cached == ("us-west-2", SimpleSource("environment")) def test_validator_called_on_default_value(self) -> None: - call_log: list[tuple[Any, str | None]] = [] + call_log: list[tuple[Any, SourceInfo | None]] = [] - def mock_validator(value: Any, source: str | None) -> Any: + def mock_validator(value: Any, source: SourceInfo | None) -> Any: call_log.append((value, source)) return value class ConfigWithDefault: - region = ConfigProperty( - "region", default_value="us-default-1", validator=mock_validator + retry_strategy = ConfigProperty( + "retry_strategy", + default_value=RetryStrategyOptions(retry_mode="standard"), + validator=mock_validator, ) def __init__(self, resolver: ConfigResolver) -> None: @@ -272,6 +320,8 @@ def __init__(self, resolver: ConfigResolver) -> None: resolver = ConfigResolver(sources=[source]) config = ConfigWithDefault(resolver) - config.region + config.retry_strategy - assert call_log == [("us-default-1", "default")] + assert call_log == [ + (RetryStrategyOptions(retry_mode="standard"), SimpleSource("default")) + ] diff --git a/packages/smithy-core/tests/unit/config/test_resolver.py b/packages/smithy-aws-core/tests/unit/config/test_resolver.py similarity index 79% rename from packages/smithy-core/tests/unit/config/test_resolver.py rename to packages/smithy-aws-core/tests/unit/config/test_resolver.py index bf016e1d2..55180b1b4 100644 --- a/packages/smithy-core/tests/unit/config/test_resolver.py +++ b/packages/smithy-aws-core/tests/unit/config/test_resolver.py @@ -2,7 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 from typing import Any -from smithy_core.config.resolver import ConfigResolver +from smithy_aws_core.config.resolver import ConfigResolver +from smithy_aws_core.config.source_info import SimpleSource class StubSource: @@ -31,7 +32,7 @@ def test_returns_value_from_single_source(self): result = resolver.get("region") - assert result == ("us-west-2", "environment") + assert result == ("us-west-2", SimpleSource("environment")) def test_returns_None_when_source_has_no_value(self): source = StubSource("environment", {}) @@ -57,7 +58,7 @@ def test_first_source_takes_precedence(self): result = resolver.get("region") - assert result == ("us-east-1", "source_one") + assert result == ("us-east-1", SimpleSource("source_one")) def test_skips_source_returning_none_and_uses_next(self): empty_source = StubSource("source_one", {}) @@ -66,7 +67,7 @@ def test_skips_source_returning_none_and_uses_next(self): result = resolver.get("region") - assert result == ("ap-south-1", "source_two") + assert result == ("ap-south-1", SimpleSource("source_two")) def test_resolves_different_keys_from_different_sources(self): instance = StubSource("source_one", {"region": "us-west-2"}) @@ -76,8 +77,8 @@ def test_resolves_different_keys_from_different_sources(self): region = resolver.get("region") retry_mode = resolver.get("retry_mode") - assert region == ("us-west-2", "source_one") - assert retry_mode == ("adaptive", "source_two") + assert region == ("us-west-2", SimpleSource("source_one")) + assert retry_mode == ("adaptive", SimpleSource("source_two")) def test_returns_non_string_values(self): source = StubSource( @@ -89,8 +90,8 @@ def test_returns_non_string_values(self): ) resolver = ConfigResolver(sources=[source]) - assert resolver.get("max_retries") == (3, "default") - assert resolver.get("use_ssl") == (True, "default") + assert resolver.get("max_retries") == (3, SimpleSource("default")) + assert resolver.get("use_ssl") == (True, SimpleSource("default")) def test_get_is_idempotent(self): source = StubSource("environment", {"region": "us-west-2"}) @@ -100,7 +101,9 @@ def test_get_is_idempotent(self): result2 = resolver.get("region") result3 = resolver.get("region") - assert result1 == result2 == result3 == ("us-west-2", "environment") + assert ( + result1 == result2 == result3 == ("us-west-2", SimpleSource("environment")) + ) def test_treats_empty_string_as_valid_value(self): source = StubSource("test", {"region": ""}) @@ -109,4 +112,4 @@ def test_treats_empty_string_as_valid_value(self): value, source_name = resolver.get("region") assert value == "" - assert source_name == "test" + assert source_name == SimpleSource("test") diff --git a/packages/smithy-core/src/smithy_core/config/__init__.py b/packages/smithy-core/src/smithy_core/config/__init__.py deleted file mode 100644 index 2a8cc831d..000000000 --- a/packages/smithy-core/src/smithy_core/config/__init__.py +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -from .resolver import ConfigResolver - -__all__ = [ - "ConfigResolver", -] diff --git a/packages/smithy-core/tests/unit/config/__init__.py b/packages/smithy-core/tests/unit/config/__init__.py deleted file mode 100644 index 04f8b7b76..000000000 --- a/packages/smithy-core/tests/unit/config/__init__.py +++ /dev/null @@ -1,2 +0,0 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 From fd6d81a367e281635998ca27c51c0bf7a100e9be Mon Sep 17 00:00:00 2001 From: Ujjwal <19787410+ubaskota@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:17:42 -0400 Subject: [PATCH 6/6] Add support for sdk_ua_app_id. (#653) * Add support for sdk_ua_app_id --- .../aws/codegen/AwsUserAgentIntegration.java | 6 +++++ .../src/smithy_aws_core/config/validators.py | 22 +++++++++++++++++++ .../tests/unit/config/test_property.py | 7 ++---- .../tests/unit/config/test_validators.py | 18 +++++++++++++++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsUserAgentIntegration.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsUserAgentIntegration.java index 423296913..df703730a 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsUserAgentIntegration.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsUserAgentIntegration.java @@ -49,6 +49,12 @@ public List getClientPlugins(GenerationContext context) { "A unique and opaque application ID that is appended to the User-Agent header.") .type(Symbol.builder().name("str").build()) .nullable(true) + .useDescriptor(true) + .validator(Symbol.builder() + .name("validate_ua_string") + .namespace("smithy_aws_core.config.validators", ".") + .addDependency(AwsPythonDependency.SMITHY_AWS_CORE) + .build()) .build(); final String user_agent_plugin_file = "user_agent"; diff --git a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py index 41b670e02..a2766ecbe 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py @@ -144,3 +144,25 @@ def validate_retry_strategy( f"retry_strategy must be RetryStrategy or RetryStrategyOptions, got {type(value).__name__}", source, ) + + +def validate_ua_string(value: Any, source: SourceInfo | None = None) -> str | None: + """Validate a User-Agent string component. + + :param value: The UA string value to validate + :param source: The source that provided this value + + :returns: The UA string or None if value is None + + :raises ConfigValidationError: If the value is not a string + """ + if value is None: + return None + if not isinstance(value, str): + raise ConfigValidationError( + "sdk_ua_app_id", + value, + f"UA string must be a string, got {type(value).__name__}", + source, + ) + return value diff --git a/packages/smithy-aws-core/tests/unit/config/test_property.py b/packages/smithy-aws-core/tests/unit/config/test_property.py index 42ea31291..4246d0602 100644 --- a/packages/smithy-aws-core/tests/unit/config/test_property.py +++ b/packages/smithy-aws-core/tests/unit/config/test_property.py @@ -107,7 +107,6 @@ def _create_config_with_validator( class ConfigWithValidator: region = ConfigProperty("region", validator=validator) - retry_strategy = ConfigProperty("retry_strategy", validator=validator) def __init__(self, resolver: ConfigResolver) -> None: self._resolver = resolver @@ -153,9 +152,7 @@ class ConfigWithComplexResolver: retry_strategy = ConfigProperty( "retry_strategy", resolver_func=mock_resolver, - default_value=RetryStrategyOptions( - retry_mode="standard", max_attempts=3 - ), + default_value=RetryStrategyOptions(retry_mode="standard"), ) def __init__(self, resolver: ConfigResolver) -> None: @@ -171,7 +168,7 @@ def __init__(self, resolver: ConfigResolver) -> None: assert isinstance(result, RetryStrategyOptions) assert result.retry_mode == "standard" - assert result.max_attempts == 3 + assert result.max_attempts is None assert source_info == SimpleSource("default") def test_validator_not_called_on_cached_access(self) -> None: diff --git a/packages/smithy-aws-core/tests/unit/config/test_validators.py b/packages/smithy-aws-core/tests/unit/config/test_validators.py index 546af5365..dbdbb7b60 100644 --- a/packages/smithy-aws-core/tests/unit/config/test_validators.py +++ b/packages/smithy-aws-core/tests/unit/config/test_validators.py @@ -8,6 +8,7 @@ validate_max_attempts, validate_region, validate_retry_mode, + validate_ua_string, ) @@ -49,3 +50,20 @@ def test_invalid_retry_mode_error_message(self) -> None: "Invalid value for 'retry_mode': 'random_mode'. retry_mode must be one " "of ('standard',), got random_mode" in str(exc_info.value) ) + + +class TestValidateUaString: + def test_allows_string(self) -> None: + assert validate_ua_string("abc123") == "abc123" + + def test_none_returns_none(self) -> None: + assert validate_ua_string(None) is None + + def test_empty_string_passthrough(self) -> None: + assert validate_ua_string("") == "" + + def test_rejects_non_string(self) -> None: + with pytest.raises(ConfigValidationError) as exc_info: + validate_ua_string(123) + + assert exc_info.value.key == "sdk_ua_app_id"