-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Pydantic-based configuration and setting objects #6321
Pydantic-based configuration and setting objects #6321
Conversation
…into feat/pydantic-config
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.
Thank you for this!
Meanwhile I also want to simplify the llm config (it doesn't need a nested version of itself): #6281 ), then it should be easier to work with.
@csmith49 Please feel free to merge this PR whenever you see fit. If there are conflicts with the other (although there might not be), no problem, I'll fix that tomorrow. |
Co-authored-by: Calvin Smith <[email protected]> Co-authored-by: Graham Neubig <[email protected]> Co-authored-by: Engel Nyst <[email protected]>
End-user friendly description of the problem this fixes or functionality that this introduces
Give a summary of what the PR does, explaining any non-trivial design decisions
This PR standardizes configuration objects to use
pydantic.BaseModel
objects over data classes. These changes were originally from #6176, but needed to be reverted in #6214 due to typing issues withSettings
(which are now fixed).Other changes:
api_key
-type fields, especially inAppConfig
andLLMConfig
, are nowSecretStr
instead ofstr
. The values can only be accessed by calling afield.get_secret_value()
function, which complicates some accesses but ensures the key won't be leaked by, e.g., serializing config objects during logging.Link of any specific issues this addresses
#6015