Add config file support for config resolution#729
Conversation
| from smithy_aws_core.config.exceptions import ConfigParseError | ||
|
|
||
| # Type aliases | ||
| type PropertyValue = str | dict[str, str] |
There was a problem hiding this comment.
[Discussion]
Instead of having ProfileProperties, a ProfileMap could have a Profile; each Profile could have ScalarProperties and GroupedProperties. The union feels a bit awkward to have to consider while coding around the StandardizedOutput object.
What do you think?
| sso_sessions: ProfileMap = {} | ||
| services: ProfileMap = {} | ||
|
|
||
| has_profile_default = False |
There was a problem hiding this comment.
[discussion/suggestion]
It feels a bit odd to have a method that has so many if statements gated behind the filetype flag. How would you feel about getting rid of that flag and instead having standardize_config_file and standardize_credentials_file methods similar to the parser methods?
| :param file_type: Whether this is a config or credentials file. | ||
| :returns: StandardizedOutput with profiles, sso_sessions, and services. | ||
| """ | ||
| profiles: ProfileMap = {} |
There was a problem hiding this comment.
[nit]
A better name for this would be SectionMap or similar since profiles is only the first of these three
| class TestMerge: | ||
| """Tests for credentials/config merge behavior.""" | ||
|
|
||
| def test_credentials_override_config_for_duplicate_key(self): |
There was a problem hiding this comment.
Isn't this assertion already covered by test_credentials_override_config?
| result = await parse_config_file(nonexistent) | ||
| assert result == {} | ||
|
|
||
| @pytest.mark.asyncio |
There was a problem hiding this comment.
This test is a superset of the test above it; we should just call parse in this test and make it all one test IMO.
SamRemis
left a comment
SamRemis
left a comment
There was a problem hiding this comment.
My bad, clicked approve by mistake. Only the test comments are blocking
ubaskota
force-pushed
the
config_file_support
branch
from
cb70998 to
27cbdd5
Compare
ubaskota
changed the base branch from
develop
to
config_integration_implementation
2 participants
Issue #, if available:
Description of changes:
Add support for parsing and merging AWS config and credentials files (
~/.aws/configand~/.aws/credentials) as part of the config resolution pipeline.Changes:
profiles/sso-sessions/services
ParsedConfigFileclass for querying the merged resultconfig-file-parser-tests.jsonandconfig-file-location-tests.json)+ unit tests for the query layer and file I/O edge casesTesting:
~/.aws/configfile to verify end-to-end parsing behaviorBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.