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

Bumped pydantic to v2 #10

Merged
merged 18 commits into from
Aug 7, 2024
Merged

Conversation

SantanaTiago
Copy link
Collaborator

@SantanaTiago SantanaTiago commented Apr 22, 2024

  • Bump pydantic to version ^2.5.3 >=2,<3
  • Added missing None in Optional fields
  • Replace deprecated methods
  • Add pydantic.v1 compatibility in pydantic.tools and validateFunction decorator
  • Replace validateFunction decorator with v2 validate_call decorator

Copy link
Collaborator

@RedRoserade RedRoserade left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Could you please check that there are no instances of deprecated methods such as the following?:

  • .parse_obj() / .parse_raw()
  • .dict() / .json()

If run the tests you should get a warnings report at the end, telling you what's deprecated.

mognet/model/result.py Outdated Show resolved Hide resolved
@SantanaTiago SantanaTiago marked this pull request as draft April 23, 2024 09:49
@SantanaTiago SantanaTiago marked this pull request as ready for review April 23, 2024 17:11
@SantanaTiago SantanaTiago marked this pull request as draft April 23, 2024 17:11
@SantanaTiago
Copy link
Collaborator Author

@RedRoserade Many thanks for the inputs. I've indeed run the tests and all pass except one with the following error

ERROR    mognet.worker.worker:worker.py:360 Could not call task function <function add_with_model at 0x7f5f70ebe170> because of a validation error
Traceback (most recent call last):
  File "/project-mognet/mognet/worker/worker.py", line 344, in _run_request
    model = validated.init_model_instance(context, *req.args, **req.kwargs)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/decorator.py", line 130, in init_model_instance
    return self.model(**values)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/main.py", line 341, in __init__
    raise validation_error
pydantic.v1.error_wrappers.ValidationError: 1 validation error for AddWithModel
model
  instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

This is related to pydantic v1 ValidatedFunction, a v1 decorator class that's used here for validation (and we using config with arbitrary_types_allowed).
Using pydantic.v1 to keep the current code/logic does not fully work and we have the error, and pydantic v2 decorator don't have a class with same/similar methods in order to keep this logic (as far as I know). Am I wrong and indeed exists a similar v2 version of this which we can apply? Or indeed this logic need to be reworked with new pydantic v2 methods (if so, I appreciate some guidelines)?

@RedRoserade
Copy link
Collaborator

@RedRoserade Many thanks for the inputs. I've indeed run the tests and all pass except one with the following error

ERROR    mognet.worker.worker:worker.py:360 Could not call task function <function add_with_model at 0x7f5f70ebe170> because of a validation error
Traceback (most recent call last):
  File "/project-mognet/mognet/worker/worker.py", line 344, in _run_request
    model = validated.init_model_instance(context, *req.args, **req.kwargs)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/decorator.py", line 130, in init_model_instance
    return self.model(**values)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/main.py", line 341, in __init__
    raise validation_error
pydantic.v1.error_wrappers.ValidationError: 1 validation error for AddWithModel
model
  instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

This is related to pydantic v1 ValidatedFunction, a v1 decorator class that's used here for validation (and we using config with arbitrary_types_allowed). Using pydantic.v1 to keep the current code/logic does not fully work and we have the error, and pydantic v2 decorator don't have a class with same/similar methods in order to keep this logic (as far as I know). Am I wrong and indeed exists a similar v2 version of this which we can apply? Or indeed this logic need to be reworked with new pydantic v2 methods (if so, I appreciate some guidelines)?

Thanks for having a look, I know that Pydantic v2 is pickier when it comes to these situations. I assume this is the failing test?

Because I find it odd that it considers AddModel to be an "arbitrary type" (it shouldn't, it's a Pydantic model):

instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

I have a feeling it's because the decorator is not using model_validate(_json), because I don't think it will automatically convert the dict to an AddModel.

I will try to have a look later today.

@SantanaTiago
Copy link
Collaborator Author

@RedRoserade Many thanks for the inputs. I've indeed run the tests and all pass except one with the following error

ERROR    mognet.worker.worker:worker.py:360 Could not call task function <function add_with_model at 0x7f5f70ebe170> because of a validation error
Traceback (most recent call last):
  File "/project-mognet/mognet/worker/worker.py", line 344, in _run_request
    model = validated.init_model_instance(context, *req.args, **req.kwargs)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/decorator.py", line 130, in init_model_instance
    return self.model(**values)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/main.py", line 341, in __init__
    raise validation_error
pydantic.v1.error_wrappers.ValidationError: 1 validation error for AddWithModel
model
  instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

This is related to pydantic v1 ValidatedFunction, a v1 decorator class that's used here for validation (and we using config with arbitrary_types_allowed). Using pydantic.v1 to keep the current code/logic does not fully work and we have the error, and pydantic v2 decorator don't have a class with same/similar methods in order to keep this logic (as far as I know). Am I wrong and indeed exists a similar v2 version of this which we can apply? Or indeed this logic need to be reworked with new pydantic v2 methods (if so, I appreciate some guidelines)?

Thanks for having a look, I know that Pydantic v2 is pickier when it comes to these situations. I assume this is the failing test?

Because I find it odd that it considers AddModel to be an "arbitrary type" (it shouldn't, it's a Pydantic model):

instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

I have a feeling it's because the decorator is not using model_validate(_json), because I don't think it will automatically convert the dict to an AddModel.

I will try to have a look later today.

Many thanks for the inputs. I've tried to make it work and tried migrate to v2 decorator validate_call methods but no success. Did you manage to have a look?

@SantanaTiago
Copy link
Collaborator Author

@RedRoserade Many thanks for the inputs. I've indeed run the tests and all pass except one with the following error

ERROR    mognet.worker.worker:worker.py:360 Could not call task function <function add_with_model at 0x7f5f70ebe170> because of a validation error
Traceback (most recent call last):
  File "/project-mognet/mognet/worker/worker.py", line 344, in _run_request
    model = validated.init_model_instance(context, *req.args, **req.kwargs)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/decorator.py", line 130, in init_model_instance
    return self.model(**values)
  File "/project-mognet/venv/lib/python3.10/site-packages/pydantic/v1/main.py", line 341, in __init__
    raise validation_error
pydantic.v1.error_wrappers.ValidationError: 1 validation error for AddWithModel
model
  instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

This is related to pydantic v1 ValidatedFunction, a v1 decorator class that's used here for validation (and we using config with arbitrary_types_allowed). Using pydantic.v1 to keep the current code/logic does not fully work and we have the error, and pydantic v2 decorator don't have a class with same/similar methods in order to keep this logic (as far as I know). Am I wrong and indeed exists a similar v2 version of this which we can apply? Or indeed this logic need to be reworked with new pydantic v2 methods (if so, I appreciate some guidelines)?

Thanks for having a look, I know that Pydantic v2 is pickier when it comes to these situations. I assume this is the failing test?
Because I find it odd that it considers AddModel to be an "arbitrary type" (it shouldn't, it's a Pydantic model):

instance of AddModel expected (type=type_error.arbitrary_type; expected_arbitrary_type=AddModel)

I have a feeling it's because the decorator is not using model_validate(_json), because I don't think it will automatically convert the dict to an AddModel.
I will try to have a look later today.

Many thanks for the inputs. I've tried to make it work and tried migrate to v2 decorator validate_call methods but no success. Did you manage to have a look?

We manage to make it work with v2 decorator validate_call

@SantanaTiago SantanaTiago marked this pull request as ready for review May 16, 2024 15:11
Copy link
Collaborator

@RedRoserade RedRoserade left a comment

Choose a reason for hiding this comment

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

Approved! Thanks for taking care of this.

Please make sure to squash the commits before merging.

@RedRoserade
Copy link
Collaborator

Also, please make sure to do a major version bump, because the changes here are backwards-incompatible (in terms of dependencies).

Signed-off-by: Tiago Santana <[email protected]>
Signed-off-by: Tiago Santana <[email protected]>
Signed-off-by: Tiago Santana <[email protected]>
…rse_raw_as with validate_json

Signed-off-by: Tiago Santana <[email protected]>
Signed-off-by: Tiago Santana <[email protected]>
Signed-off-by: Tiago Santana <[email protected]>
@dolfim-ibm dolfim-ibm merged commit b171610 into DS4SD:main Aug 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants