-
Notifications
You must be signed in to change notification settings - Fork 28
Replace raw source strings with typed SourceInfo dataclasses #652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
ubaskota marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we introduced the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its not possible to do this now due to the nature of our tests, so added a TODO.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point of this PR is to make sure that the source's name is one of a small number of acceptable values. If we aren't checking that those values are valid when we create the source via strict typing, the point of this PR is moot. If we can't properly test code, sometimes that may mean that it's poorly written code. Often, it means we are either testing things that aren't valuable or we haven't thought of the right way to test them. The tests are here to make sure that the code works, not the other way around. |
||
|
|
||
|
|
||
| @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] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also use the new enum: |
||
|
|
||
|
|
||
| SourceInfo = SimpleSource | ComplexSource | ||
SamRemis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Uh oh!
There was an error while loading. Please reload this page.