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

Class Initialisation Validation Kwarg #227

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

AndrewRiggs-Atkins
Copy link

@AndrewRiggs-Atkins AndrewRiggs-Atkins commented Jan 17, 2022

As discussed in issue #134, this pr enables pydantic validation to be run on object instantiation when the class is set as a table, instead of only when being commited into the database.

main.py > SQLModelMetaclass > new > line ~307

config_validate = get_config("validate")
if config_validate is True:
    # If it was passed by kwargs, ensure it's also set in config
    new_cls.__config__.validate = config_validate
    for k, v in new_cls.__fields__.items():
        col = get_column_from_field(v)
        setattr(new_cls, k, col)

main.py > SQLModel > init > line ~517

if (
    (not getattr(__pydantic_self__.__config__, "table", False)
    or getattr(__pydantic_self__.__config__, "validate", False)) # Added validate
    and validation_error
):
    raise validation_error

usage

class Hero(SQLModel, table=True, validate=True): # Added validate

    id: int = Field(primary_key=True, nullable=False)
    number: int

    @validator('number')
    def less_than_100(cls, v):
        if v > 100:
            raise ValueError('must be less than 100')
        return v

When validate is disabled, validation runs on commit as usual, with it enabled, validation runs on object initialisation. Works for pydanic @validate functions as well as others such as max_length

@AndrewRiggs-Atkins
Copy link
Author

@tiangolo Would it be possible to get the workflows run on this pr?

@AndrewRiggs-Atkins
Copy link
Author

@tiangolo Would it be possible to get this PR reviewed and workflows run?

@github-actions
Copy link

📝 Docs preview for commit 7fd81a1 at: https://630aa502b736cd5736509bdb--sqlmodel.netlify.app

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Base: 98.49% // Head: 97.54% // Decreases project coverage by -0.96% ⚠️

Coverage data is based on head (7fd81a1) compared to base (ea79c47).
Patch coverage: 28.57% of modified lines in pull request are covered.

❗ Current head 7fd81a1 differs from pull request most recent head 4499f96. Consider uploading reports for the commit 4499f96 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
- Coverage   98.49%   97.54%   -0.96%     
==========================================
  Files         185      185              
  Lines        5856     6109     +253     
==========================================
+ Hits         5768     5959     +191     
- Misses         88      150      +62     
Impacted Files Coverage Δ
sqlmodel/main.py 83.47% <28.57%> (ø)
sqlmodel/sql/expression.py 87.17% <0.00%> (-10.26%) ⬇️
tests/test_missing_type.py 92.30% <0.00%> (-1.03%) ⬇️
tests/conftest.py 100.00% <0.00%> (ø)
tests/test_enums.py 100.00% <0.00%> (ø)
tests/test_validation.py 100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/teams/tutorial001.py 100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/delete/tutorial001.py 100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/update/tutorial001.py 100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/read_one/tutorial001.py 100.00% <0.00%> (ø)
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link

📝 Docs preview for commit d295a05 at: https://639cfbfcd8175d1a762d5fda--sqlmodel.netlify.app

@ryanjdillon
Copy link

I just ran across this issue myself, so it would be great to get this fix merged.

Looks like it is just some lint errors in the actions failures.

Any plan to get this rolled in? Thanks!

@github-actions
Copy link

📝 Docs preview for commit 4869ea2 at: https://63ecc07fbac3c3005ddacbb8--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit 8857b75 at: https://63ecc372a5cfbc008c072238--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit d37b852 at: https://63ecc478bac3c3007cdad284--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit d49a0d6 at: https://63ecc579a5cfbc00570722fb--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit 115de85 at: https://63ecc982bac3c305d0daccb5--sqlmodel.netlify.app

@github-actions
Copy link

📝 Docs preview for commit 4499f96 at: https://63ecd2b82960b411061a9718--sqlmodel.netlify.app

@AndrewRiggs-Atkins
Copy link
Author

I just ran across this issue myself, so it would be great to get this fix merged.

Looks like it is just some lint errors in the actions failures.

Any plan to get this rolled in? Thanks!

I've resolved the conflicts and the pr now passes the latest checks.

@tiangolo Are we able to get a go/no-go on this PR, been sitting around for over a year now so be good to get a final decision on whether it should be updated, merged, or closed.

@tiangolo tiangolo added the feature New feature or request label Oct 22, 2023
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

3 participants