Add CLI App Support #389
Add CLI App Support #389
Conversation
![hyperlint-ai[bot]](https://avatars.githubusercontent.com/in/718456?s=60&v=4){kind=small}
| this_foo: str | ||
|
|
||
|
|
||
| print(Settings(_cli_parse_args=['--this_foo', 'is such a foo']).model_dump()) |
There was a problem hiding this comment.
Fixed suggestion of using private param _cli_parse_args. The "formal" equivalent is now:
CliApp.run(Settings, cli_args=['--this_foo', 'is such a foo']).model_dump()
| @@ -1211,13 +1287,15 @@ cli_settings = CliSettingsSource(Settings, root_parser=parser) | |||
|
|
|||
| # Parse and load CLI settings from the command line into the settings source. | |||
| sys.argv = ['example.py', '--food', 'kiwi', '--name', 'waldo'] | |||
| print(Settings(_cli_settings_source=cli_settings(args=True)).model_dump()) | |||
There was a problem hiding this comment.
Fixed suggestions of using private param _cli_settings_source for its formal equivalent.
| @@ -33,7 +40,6 @@ class SettingsConfigDict(ConfigDict, total=False): | |||
| env_parse_enums: bool | None | |||
| cli_prog_name: str | None | |||
| cli_parse_args: bool | list[str] | tuple[str, ...] | None | |||
| cli_settings_source: CliSettingsSource[Any] | None | |||
There was a problem hiding this comment.
This was a bug. It's not possible to provide cli_settings_source as a configuration setting as the settings_cls cannot be defined (circular dependency).
|
@hramezani this is ready for review. I removed most the scope originally discussed in #335 to avoid breaking changes and keep maintenance light. i.e., I think any remaining items can depend on demand from the community. |
|
Thanks @kschwab for this PR. I have some concerns about the cli settings source, it has a lot of code and I think maintaining this amount of code would be hard in the future. Specially about |
|
@hramezani, yes, this was requested and discussed in #335 and #336. It is related to discussion we had on get_subcomand, where this functionality for CLI is somewhat standard. I think drawing the line here for feature support makes sense. Also, I understand the LOC concern for |
|
@kschwab Don't we need more tests for this patch? I can't see any test for |
|
@hramezani it's mostly a superficial commit. The doc tests cover all happy paths, and I added tests in |
|
@hramezani, I added more tests specific to |
|
Thanks @kschwab. I will check it next week |
|
Thanks @kschwab |
Adds utility functions for running CLI applications.
See documentation for details.