Skip to content

Add a config property descriptor along with a custom resolver and validators#642

Merged
ubaskota merged 7 commits intosmithy-lang:config_resolution_mainfrom
ubaskota:config_property
Feb 28, 2026
Merged

Add a config property descriptor along with a custom resolver and validators#642
ubaskota merged 7 commits intosmithy-lang:config_resolution_mainfrom
ubaskota:config_property

Conversation

@ubaskota
Copy link

Issue #, if available:

This PR adds a ConfigProperty descriptor that serves as the central instrument for configuration resolution. The descriptor is responsible for resolving config values from sources, applying custom resolvers for complex resolutions, validating values, caching them, and facilitating value updates.

The ConfigProperty descriptor implements the Python descriptor protocol to provide:

  • Lazy resolution
  • Validation
  • Caching
  • Custom resolver
  • Source tracking

Initial config variables:

  • region - demonstrates simple resolution
  • retry_strategy - demonstrates complex resolution (aggregates retry_mode and max_attempts)

These two variables ensure that the design handles both simple and complex resolution.

Tests

  • Verified that the unit tests for config property, validators and custom revolvers pass successfully.

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

@ubaskota ubaskota requested a review from a team as a code owner February 24, 2026 05:29
# Get max_attempts
max_attempts, attempts_source = resolver.get("max_attempts")

if retry_mode is None or max_attempts is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ignoring one of these values if both aren't set. It's valid for a customer to only set retry_mode and not max_attempts and expect that to be respected.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think of a scenario where we need to resolve retry_strategy, but the resolver only finds retry_mode in the sources without max_attempts. Should we mix the resolved retry_mode with a default max_attempts value? This approach doesn't make sense because when a client needs to resolve retry_strategy, it will have a default retry_strategy value configured. To me it doesn’t make sense to mix a partially resolved strategy with default components. Instead, if either component (retry_mode or max_attempts) is missing, we should use the complete default retry_strategy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I agree with that; I should be able to set either mode OR strategy and have that be reflected. That's how this input works today - you can set only one of the two inputs on the RetryStrategyOptions object.

As a user, I can trust the SDK to pick the right number of max attempts, but realize that I'm likely to get throttled so I'd want to choose adaptive retry mode. If I only set AWS_RETRY_MODE=adaptive in the environment, that needs to be respected.

While I don't think there's a scenario where it would make sense to ignore the user input here, even if there were, we would at least need to raise an error or warning to tell the customer that we are ignoring their input. This would ignore it silently.

self,
key: str,
validator: Callable[[Any, str | None], Any] | None = None,
resolver_func: Callable[[ConfigResolver], tuple[Any, str | None]] | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up, I'd like to talk about getting the source to be an enum instead of a string

# Get max_attempts
max_attempts, attempts_source = resolver.get("max_attempts")

if retry_mode is None or max_attempts is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I agree with that; I should be able to set either mode OR strategy and have that be reflected. That's how this input works today - you can set only one of the two inputs on the RetryStrategyOptions object.

As a user, I can trust the SDK to pick the right number of max attempts, but realize that I'm likely to get throttled so I'd want to choose adaptive retry mode. If I only set AWS_RETRY_MODE=adaptive in the environment, that needs to be respected.

While I don't think there's a scenario where it would make sense to ignore the user input here, even if there were, we would at least need to raise an error or warning to tell the customer that we are ignoring their input. This would ignore it silently.

max_attempts = validate_max_attempts(max_attempts, attempts_source)

options = RetryStrategyOptions(
retry_mode=retry_mode or "standard", # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
retry_mode=retry_mode or "standard", # type: ignore
retry_mode=retry_mode

super().__init__(msg)


def validate_host_label(host_label: Any, source: str | None = None) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A host label has to be a string, right?

Suggested change
def validate_host_label(host_label: Any, source: str | None = None) -> str:
def validate_host_label(host_label: str, source: str | None = None) -> str:

if self.resolver_func:
value, source = self.resolver_func(obj._resolver)
else:
value, source = obj._resolver.get(self.key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As another follow up, we should expose each source from the resolver so they can be called directly, for example for use in environment only configs like the bedrock token

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third follow up - we should have some caching on the resolver as well and make sure they share the config resolver.

@ubaskota ubaskota merged commit eb22199 into smithy-lang:config_resolution_main Feb 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants