-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 Secret base type #8519
Add Secret base type #8519
Conversation
CodSpeed Performance ReportMerging #8519 will not alter performanceComparing Summary
|
Thanks for your work on this! Looking great thus far. Just curious - are there any other types that you'd like to see The tests look great 🚀. Now that we have more than just 2 Would it be ok if I pull your branch down and try a few variations on the code that you've added thus far? |
Sure! This was a quick implementation I've tried. I'm not yet fully familiar with V2 way of defining new types.
what do you think? |
Great, I'll pull it down soon :). I can see the argument for that, though I think it spirals in that we'd have to support secret behavior for all types in that case. I think for now we'll probably want to stick with explicitly defined secret types. |
Makes sense, let me know how can I assist, I'd really like to contribute with Pydantic :) |
Sure thing! Alright, I've pushed some changes in regards to how we inherit from Currently, we don't support the validation of from pydantic import SecretStr, TypeAdapter, ValidationError
s_invalid = SecretStr(123)
print(f"type: {type(s_invalid.get_secret_value())}, value: {s_invalid.get_secret_value()}")
#> type: <class 'int'>, value: 123
try:
ta = TypeAdapter(SecretStr)
s_validated = ta.validate_python(123)
except ValidationError as e:
print(f"ValidationError: {e}")
"""
ValidationError: 1 validation error for json-or-python[json=function-after[SecretStr(), str],python=union[is-instance[SecretStr],function-after[SecretStr(), str]]]
Input should be a valid string [type=string_type, input_value=123, input_type=int]
For further information visit https://errors.pydantic.dev/2.6/v/string_type
""" I've removed the support for this for I think for now, we should stick to not supporting that behavior, as implementation would require some complex changes (I don't think we'd want to import the As a side note, this issue is related to the above discussion: This issue is related: #7353. |
I'll make sure everything is working in terms of linting / testing, then hand this back to you :). |
Could you please update this example with Other than that, I think this PR is ready for review, so I'll mark it as such. I've labeled many of our other issues as Thanks a bunch for your help with this - we're super excited to adopt this feature :). |
Please review |
Thanks! I'll take a look at the docs and will probably try to make a secret Enum! |
Sounds great! Ping me for a review when you're done :). |
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.
idea looks good, I wonder if we're be better to implement this via
SecretDate = Annotated[date, Secret(repr=lambda value: '****/**/**')]
So we can easily add more types, even if we can't use the Secret
type for SecretStr
and SecretBytes
until V3 for compatibility reasons.
Agreed. @conradogarciaberrotaran, are you interested in adding this type, or would you like me to? I can also help you get a start if you need 👍. Also, thanks for making the changes that @samuelcolvin requested. Your changes look good. |
Just pushed missing tests and doc updates! |
please review |
Update, I don't think this is required for this PR:
It'd be a nice feature down the line, but you can currently apply |
I believe all relevant concerns have been addressed here and this is ready for review :). |
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.
I haven’t reviewed the tests or implementation in detail but in general I really like this approach!
Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>
…ciaberrotaran/pydantic into add-secret-date-field
Nice work on helping implement this new feature! It'll be one of the highlights of our 2.7 release :). |
Thanks for your help! I hope I can contribute more in the future. |
Change Summary
Related issue number
Closes #8441
Checklist
Selected Reviewer: @sydney-runkle