-
-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add CLI Settings Source #214
Conversation
Will update to be individualized. This will also help improve generated help text, which was a concern when using class doc strings.
I am most of the way through but hit this limitation in the underlying environment variable parsing:
Obviously, this would be very nice to have from a CLI perspective (e.g., nested lists etc.). I didn't know this was the case until I started writing nested tests. Do you have suggestions on what the best course of action is here? Ideally, I would love for it to be handled in environment parsing. If not, from CLI perspective the next easiest path would be nested dictionaries of strings. Is that something I can throw to Pydantic and have a reasonable expectation it will sort it out? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
+ Coverage 97.69% 98.65% +0.96%
==========================================
Files 5 5
Lines 347 597 +250
Branches 76 152 +76
==========================================
+ Hits 339 589 +250
Misses 6 6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Thanks @kschwab for this patch 🙏 Regarding JSON parsing issue please take the easiest path. Also, you need to add documentation for this and show how to use this new feature. |
Sorry @hramezani, I should have been more specific with my question. I am at a fork and am looking for guidance on which direction to take. Is there any background on why nested JSON isn't supported in My preference would be to solve it there so that the CLI and ENV are consistent. Just trying to ascertain if that is a reasonable direction or not. |
I think the comment that you brought from the doc is an old one and related to the first time that nested env support added to Pydantic V1. Here is my understanding from the comment: class SubValue(BaseModel):
v4: str
v5: int
class TopValue(BaseModel):
v1: str
v2: str
v3: str
sub: SubValue
class Cfg(BaseSettings):
v0: str
top: TopValue
model_config = SettingsConfigDict(env_nested_delimiter='__') you only can provide JSON values for
and it is not possible to provide JSON values for sub fields like:
But, you can see that passing JSON values for sub fields also works. |
Got it. So it's either a bug or something invalid in the test case. Below is an example of the issue I am seeing: import os
from pydantic import BaseModel
from typing import Optional, List
from pydantic_settings import BaseSettings
class Child(BaseModel):
num_list: Optional[List[int]] = None
class Cfg(BaseSettings, env_nested_delimiter='__'):
child: Optional[Child] = None
os.environ['CHILD__NUM_LIST'] = '[1,2,3]'
print(Cfg().model_dump())
'''
Traceback (most recent call last):
File "/home/kschwab/pydantic-settings/example.py", line 23, in <module>
print(Cfg().model_dump())
File "/home/kschwab/pydantic-settings/pydantic_settings/main.py", line 93, in __init__
super().__init__(
File "/home/kschwab/.local/lib/python3.10/site-packages/pydantic/main.py", line 171, in __init__
self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 1 validation error for Cfg
child.num_list
Input should be a valid list [type=list_type, input_value='[1,2,3]', input_type=str]
For further information visit https://errors.pydantic.dev/2.6/v/list_type
''' If you note the |
It's a bug. Here is the related issue |
Well, not great that it's a bug, but good in the context of this PR since it allows us to keep using I'll continue on with rest of documentation, cleanup, etc. and loop back to it if it's still there. |
The bulk of the functionality is now complete. Will continue to work on cleanup, comments, docs etc. over next few days. The only key remaining feedback items needed for any adjustments are:
Outside of the above, I think everything else is pretty straightforward and aligns with precedence already set by environment variables. |
Hi @hramezani, need some help with resolving the below:
|
You can use ```py test="skip" to skip running on those blocks
This will be available on
Not right now. |
Yeah. that was my first thought. but after talking with the team we decided to have the CLI source like other sources. |
No worries!
I haven't looked at the latest toml, yaml source changes and how it was integrated, but will try and align with it if possible. I don't think there should be any issues. The only thing that comes to mind is the external parser support, but I think it should be fine. We'll make the updates and see where we land 🙂 |
@hramezani I've addressed the issues raised. For case-insensitive matching, this is not supported on external parsers, it is only supported on the internal parser. |
Thanks @kschwab for addressing the issues. I will check it later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much @kschwab for working on this, and so sorry for my slow response 🙏 .
Overall, I think this is AWESOME, and I can't wait to use it.
I have one minor suggestion for an improvement from trying this out (can be fixed now or in future):
It would be nice if the --help
output differentiated between required and not-required fields, and I guess showed the default value for non-required fields.
E.g. for this code:
from pydantic_settings import BaseSettings
from devtools import debug
class Settings(BaseSettings, use_attribute_docstrings=True, cli_parse_args=True):
"""
Settings for the application.
"""
foo: str
"""Set the foo value."""
bar: int = 123
s = Settings()
debug(s)
The help output was:
usage: demo.py [-h] [--foo str] [--bar int]
Settings for the application.
options:
-h, --help show this help message and exit
--foo str Set the foo value. Required
--bar int Optional, default=123.
E.g. we append ' Required'
or ' Optional, default={default!r}'
to the description, or use that as the description if there is none.
Thanks @samuelcolvin for the review. I added the differentiation with the last commit. Required fields are marked as
|
@kschwab Thanks for your update. I am going to merge the PR if you are done with your updates. It will be released soon 🚀 |
Thanks @kschwab for your great effort here ❤️ |
Thanks @hramezani! Glad to see it go in 👍🏾 |
Resolves #209.
This PR implements a
CliSettingsSource
with the following main features outlined below. Refer to docs/index.md for more details.Current Examples
Fields
_cli_parse_args
can accept arg array orbool
set toTrue
forsys.argv
Lists
--field='[1,2,3]'
--field 1 --field 2 --field 3
--field=1,2,3
Dictionaries
--field='{"k1": 1, "k2": 2}'
--field k1=1 --field k2=2
--field k1=1,k2=2 --field k3=3 --field '{"k4: 4}'
etc.Subcommands and Positionals
_cli_prog_name
has same meaning as in argparseCliSubCommand
annotation for marking subcommandsCliPositionalArg
annotation for marking positional arguments