Skip to content

Update the codegen files that generate config.py to support config resolution.#646

Open
ubaskota wants to merge 2 commits intosmithy-lang:config_resolution_mainfrom
ubaskota:config_resolve_client_codegen
Open

Update the codegen files that generate config.py to support config resolution.#646
ubaskota wants to merge 2 commits intosmithy-lang:config_resolution_mainfrom
ubaskota:config_resolve_client_codegen

Conversation

@ubaskota
Copy link

Issue #, if available:

Description of changes:
This PR updates the Python client code generator to support descriptor-based configuration resolution with source tracking. The changes enable generated config.py files to use the descriptor pattern for resolution of configuration properties from multiple sources with proper precedence and caching.

Tests:
Verified that the newly generated code in config.py has all the expected changes as shown below:

  • Verified that it has all the required import statements:
  • Verified that the changes for region and retry_strategy are generated:
    region = ConfigProperty(
        "region", validator=validate_host, default_value="us-east-1"
    )
    retry_strategy = ConfigProperty(
        "retry_strategy",
        validator=validate_retry_strategy,
        resolver_func=resolve_retry_strategy,
        default_value=RetryStrategyOptions(retry_mode="standard", max_attempts=3),
    )
  • Verified that the changes to handle instance values are generated.
       descriptor_keys = ["region", "retry_strategy"]
        for key in descriptor_keys:
            value = locals().get(key)
            if value is not None:
                setattr(self, key, value)
  • Verified that the function to get the source of a config value is generated.
    def get_source(self, key: str) -> str | None:
       .......
        cached = self.__dict__.get(f"_cache_{key}")
        return cached[1] if cached else None

All tests passed successfully, confirming that the descriptor-based resolution works as expected . Validated configuration resolution using the generated code with the following scenarios:

**Region resolution**
- region uses the default value when its not available in the config sources 
- region uses the value from the instance 
- region uses the value set in environment variable 

**Retry Strategy resolution**
- retry_strategy uses the default value when not available in the config sources 
- retry_strategy uses the value set in instance 
- retry_strategy uses the value set in environment 

**Other**
- Source name for complex resolution is as expected
- Config value set in instance overrides the values set in environment 

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 26, 2026 18:57

// Add _resolver declaration
writer.write("_resolver: ConfigResolver");
writer.writeDocs("Shared resolver for all Config instances.", context);
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
writer.writeDocs("Shared resolver for all Config instances.", context);
writer.writeDocs("Resolver for iterating through config sources to retrieve keys.", context);

This is no longer shared, right?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, its not shared. I realized it after publishing this PR and decided to wait until the first round of review is completed.

writer.write("");

// Then, handle descriptor properties efficiently with a loop
var descriptorProperties = properties.stream()
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 in the generic codegen and is not aws specific - are we adding descriptors and everything to white label SDKs?

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.

2 participants