Add support for config resolver by ubaskota · Pull Request #738 · smithy-lang/smithy-python

ubaskota · 2026-07-13T05:19:24Z

Issue #, if available:

Description of changes:
This change implements the async config resolution pipeline with AsyncAwsConfig using a resolve() classmethod as the only construction path. It adds SharedConfigContext for holding env vars and memoized config file data, FieldSpec for per-field resolution metadata, and provenance tracking via ConfigSource. Resolvers for region and retry_strategy_options are included along with validators for both fields.

Testing:

Added unit and end-to-end tests covering env var resolution, profile resolution, explicit overrides, defaults, provenance tracking, validator enforcement, and construction blocking, and verified that they all pass.
All existing tests pass

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Alan4506 · 2026-07-16T13:51:32Z

+    result = await _resolve_str(ctx, env_vars=env_vars, profile_keys=profile_keys)
+    if result.value is UNSET:
+        return Resolved(value=UNSET, source=ConfigSource.DEFAULT)  # type: ignore[arg-type]
+    if result.value is None:


Nit: Is this dead code? _resolve_str may never return Resolved with a None value?

Alan4506 · 2026-07-16T14:04:16Z

+        sources_found.append(attempts_result.source)
+
+    source = (
+        _strongest_source(*sources_found) if sources_found else ConfigSource.DEFAULT


Since we only have source_of("retry_strategy_options"), do we lose the ability to tell where retry_mode vs max_attempts each came from? E.g. retry_mode from env but max_attempts from profile would both just report ENV? I forgot if we discussed this yesterday - please refresh my memory if we did. I'm curious about this because it may affect another PR - which depends on provenance tracking, and needs to know if the max_attempts itself has been set by customer or not. If we've decided to keep provenance at the options level (rather than per-field) for now, then the other PR may consider a different way to determine if the customer has set the max_attempts manually.

Yes, for composite resolvers like retry_strategy_options, we report the strongest source across the fields that compose it (example: if retry_mode comes from ENV and max_attempts from PROFILE, the source is ENV). This is something the team decided upon. If your other PR needs to know specifically whether max_attempts was user-set, we can discuss that.

# Conflicts: # packages/smithy-aws-core/src/smithy_aws_core/config/exceptions.py

jonathan343

Thanks Ujjwal. Overall, everything seems functional after testing with different direct, env var, and config file setups.

I let some comments calling out some gaps and bugs in the implementation. Let me know if you have any questions. I'll continue to think about if this is the optimal interface we want to expose.

jonathan343 · 2026-07-21T18:41:26Z

+        assert result.source == ConfigSource.ENV
+
+    @pytest.mark.asyncio
+    async def test_uses_defaults_when_nothing_found(self):


There are a few tests in this file that build SharedConfigContext(env={}) with no file_path overrides. This causes them to eventually read the real ~/.aws/config and ~/.aws/credentials files on the runners machine. If the machine's [default] profile sets retry_mode or max_attempts, the assertions will fail.

This issue affects the following tests:

test_uses_defaults_when_nothing_found

test_partial_resolution_uses_defaults_for_missing

test_max_attempts_unset_when_not_found.

jonathan343 · 2026-07-21T18:44:55Z

+        assert config.region == "us-west-2"
+
+    @pytest.mark.asyncio
+    async def test_resolves_retry_from_env(self):


This test is also environment dependent. It relies on the test runner's machine to define region = ... in the ~/.aws/config file. If it doesn't it will fail with the following error:

smithy_aws_core.config.exceptions.ConfigError: Invalid value for 'region': None. Region is required and must be set.

Tests are run on various different dev, CI, and containerized setups, so they need to be deterministic and not reliant on the environment they're run in.

jonathan343 · 2026-07-21T18:55:03Z

+    sources_found: list[ConfigSource] = []
+
+    if mode_result.value is not UNSET and mode_result.value is not None:
+        kwargs["retry_mode"] = mode_result.value


We need to validate that the configured retry_mode value is one we accept/support. I'd recommend validating against typing.get_args(RetryStrategyType) or something similar so it stays aligned with the actual supported list. If something is configured that isn't in that list, we should raise an error.

Here is an example of me being able to configure a mode that doesn't exist:

config file:

# ~/.aws/config [default] retry_mode = fake-mode

(smithy-python) $ python -m asyncio asyncio REPL 3.12.13 (main, Mar 25 2026, 03:16:06) [Clang 22.1.1 ] on darwin Use "await" directly instead of "asyncio.run()". Type "help", "copyright", "credits" or "license" for more information. >>> import asyncio >>> from smithy_aws_core.config.aws_config import AsyncAwsConfig >>> config = await AsyncAwsConfig.resolve() >>> config AsyncAwsConfig(region='us-west-2', retry_strategy_options=RetryStrategyOptions(retry_mode='fake-mode', max_attempts=None))

For reference, this is what botocore raises:

botocore.exceptions.InvalidRetryModeError: Invalid value provided to "mode": "fake-mode" must be one of: ('legacy', 'standard', 'adaptive')

jonathan343 · 2026-07-21T18:57:33Z

+        sources_found.append(mode_result.source)
+
+    if attempts_result.value is not UNSET and attempts_result.value is not None:
+        kwargs["max_attempts"] = attempts_result.value


I also think we should validate the these ranges. Here is an example of me sucessfully configuring max_attempts = -1 which shouldn't be allowed:

config file:

# ~/.aws/config [default] max_attempts = -1

(smithy-python) $ python -m asyncio asyncio REPL 3.12.13 (main, Mar 25 2026, 03:16:06) [Clang 22.1.1 ] on darwin Use "await" directly instead of "asyncio.run()". Type "help", "copyright", "credits" or "license" for more information. >>> import asyncio >>> from smithy_aws_core.config.aws_config import AsyncAwsConfig >>> config = await AsyncAwsConfig.resolve() >>> config AsyncAwsConfig(region='us-west-2', retry_strategy_options=RetryStrategyOptions(retry_mode='standard', max_attempts=-1))

Botocore error for reference:

botocore.exceptions.InvalidMaxRetryAttemptsError: Value provided to "max_attempts": -1 must be an integer greater than or equal to 1.

jonathan343 · 2026-07-21T19:25:46Z

+_REGION_PATTERN = re.compile(r"^(?![0-9]+$)(?!-)[a-zA-Z0-9-]{1,63}(?<!-)$")
+
+
+def validate_region(value: object) -> None:


It seems like we're only validating on the initial AsyncAwsConfig.resolve(...) call. When users set values directly after resolution, I feel like we should be revalidating.

Example 1: config rejects a bad value at resolution time:

(smithy-python) $ python -m asyncio asyncio REPL 3.12.13 (main, Mar 25 2026, 03:16:06) [Clang 22.1.1 ] on darwin Use "await" directly instead of "asyncio.run()". Type "help", "copyright", "credits" or "license" for more information. >>> import asyncio >>> from smithy_aws_core.config.aws_config import AsyncAwsConfig >>> config = await AsyncAwsConfig.resolve(region="bad-value!") Traceback (most recent call last): File "/Users/gytndd/.local/share/uv/python/cpython-3.12.13-macos-aarch64-none/lib/python3.12/concurrent/futures/_base.py", line 456, in result return self.__get_result() ^^^^^^^^^^^^^^^^^^^ File "/Users/gytndd/.local/share/uv/python/cpython-3.12.13-macos-aarch64-none/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result raise self._exception File "<console>", line 1, in <module> File "/Users/gytndd/dev/GitHub/temp6/smithy-python/packages/smithy-aws-core/src/smithy_aws_core/config/aws_config.py", line 96, in resolve await instance._resolve_fields(overrides) File "/Users/gytndd/dev/GitHub/temp6/smithy-python/packages/smithy-aws-core/src/smithy_aws_core/config/aws_config.py", line 156, in _resolve_fields spec.validator(getattr(self, field_name)) File "/Users/gytndd/dev/GitHub/temp6/smithy-python/packages/smithy-aws-core/src/smithy_aws_core/config/validators.py", line 26, in validate_region raise ConfigError( smithy_aws_core.config.exceptions.ConfigError: Invalid value for 'region': 'bad-value!'. Must be a valid AWS region identifier.

Example 2: we config a valid region, then successfully overwrite with a bad region:

>>> config = await AsyncAwsConfig.resolve(region="us-east-1") >>> config.region 'us-east-1' >>> config.region = "bad-value!" >>> config.region 'bad-value!'

jonathan343 · 2026-07-21T19:37:50Z

+    except (ValueError, TypeError):
+        raise ConfigError(
+            f"Invalid integer value {result.value!r} for config key. "
+            "Expected a valid integer."
+        )


nit - We should catch the error and raise from it:

except (ValueError, TypeError) as e: raise ConfigError(...) from e

jonathan343 · 2026-07-21T19:46:35Z

+        cls,
+        *,
+        profile: str | None = None,
+        env: Mapping[str, str] | None = None,


It doesn't seem like we're gaining the full value of env since it doesn't get propagated to the methods lower in the call stack. Specifically, it gets lost through parsed_profiles() → load_config() → _resolve_config_paths() which just use os.environ.

Could we thread ctx.env all the way through load_config/_resolve_config_paths (an optional env param defaulting to os.environ) so all env lookups share one source?

I have some concerns about this after investigating an env mapping override for our new credential chain.

Does the env mapping provide any value outside of environment injection for tests? Correct me if I'm wrong, but I don't think we want or need to expose this for customers to provide custom mappings in the config. Should we not always use the os environment?

If the above is true, keeping this adds unnecessary plumbing which can be replaced by monkeypatch in tests.

jonathan343 · 2026-07-21T19:49:36Z

+        :param credentials_file_path: Override path for credentials file.
+        """
+        self._env: Mapping[str, str] = env if env is not None else os.environ
+        self._fs: FileSystem = fs if fs is not None else DefaultFileSystem()


I don't really get why this file abstraction was introduced. It seems to be stored here but is never used. Can you explain to me why this was introduced and what value it's supposed to bring?

ubaskota requested a review from a team as a code owner July 13, 2026 05:19

Alan4506 reviewed Jul 16, 2026

View reviewed changes

Add support for config resolver

feb09c2

# Conflicts: # packages/smithy-aws-core/src/smithy_aws_core/config/exceptions.py

ubaskota force-pushed the config_resolver_implementation branch from 34efc14 to feb09c2 Compare July 21, 2026 17:55

jonathan343 requested changes Jul 21, 2026

View reviewed changes

arandito deleted the branch smithy-lang:config_integration_implementation July 22, 2026 17:50

arandito closed this Jul 22, 2026

		_REGION_PATTERN = re.compile(r"^(?![0-9]+$)(?!-)[a-zA-Z0-9-]{1,63}(?<!-)$")


		def validate_region(value: object) -> None:

Uh oh!

Conversation

ubaskota commented Jul 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathan343 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arandito Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arandito Jul 22, 2026 •

edited

Loading