Skip to content
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 max_length validation for database models and input data #1233

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

estebanx64
Copy link
Collaborator

No description provided.

@estebanx64 estebanx64 changed the title Add max_length validation for database models and input data 🗃️ Add max_length validation for database models and input data Jun 17, 2024
Copy link
Owner

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Just a small request. 🙏



# Properties to receive via API on creation
class UserCreate(UserBase):
password: str
password: str = Field(min_length=8, max_length=40)
Copy link
Owner

@tiangolo tiangolo Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update the max length of passwords to the same of the other text fields, with 255?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be the max lenght?

Copy link
Owner

@tiangolo tiangolo Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, right @PhilippWu ! (just edited it)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatting internally, @estebanx64 convinced me to leave this with 40. 😅

Copy link

@glendigity glendigity Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tiangolo or @estebanx64 , would you be able to share the logic on the 40? Sounds counterintuitive to me, so something to learn.

@tiangolo tiangolo added the feature New feature or request label Jun 18, 2024
Copy link
Owner

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! 🚀

@tiangolo tiangolo merged commit d421c90 into master Jun 19, 2024
3 checks passed
@tiangolo tiangolo deleted the FL-397-set-varchar-255 branch June 19, 2024 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants