Add support for pydantic v2 #795

JacobHayes · 2023-10-01T22:10:59Z

Pull Request Checklist

Fixes Support Pydantic v2 #713
Tests added
Documentation/examples added
Good commit messages and/or PR title

Description of PR
WIP, but wanted to get the bulk of it up for any feedback while I:

add a test run in CI with pydantic v1 installed (to ensure existing users are covered - they passed manually)
add a couple more tests and try to support serializing v1 and v2 user-defined models (not just the v1 models from either version - I think this should be possible).
update the model generation to update the Field import

This PR updates hera to support use with either pydantic v1 or v2 installed. All hera internal code now imports pydantic objects from hera.shared._pydantic (extended and renamed from _base_model). This module ensures that we're always using v1 compatible objects, which allows us to support codebases installing either pydantic v1 or v2 (although the models passed to Hera must be v1 for pydantic v1 and v2 for pydantic v2).

JacobHayes · 2023-10-01T22:12:59Z

Sorry for the large PR w/o much prior discussion - I think a lot of it is pretty rote transformations though (updating imports). This one might also be a bit easier to review commit-by-commit.

matty-rose · 2023-10-10T03:50:38Z

hey @JacobHayes bit of a driveby comment but i'd done a little digging as well into introducing pydantic v2 compatibility here and ran into the exact issues with the codegen stuff seen in the CI failures here - the commit from this PR - JacobHayes#1 i think should help fix those. hope this helps a little, super keen to see this PR land!

JacobHayes · 2023-10-11T14:47:07Z

Much appreciated @matty-rose!

codecov · 2023-10-11T15:57:06Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (826cebb) 79.7% compared to head (2b8ed33) 79.7%.

Files	Patch %	Lines
src/hera/shared/serialization.py	69.2%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #795   +/-   ##
=====================================
  Coverage   79.7%   79.7%           
=====================================
  Files         45      45           
  Lines       3706    3724   +18     
  Branches     750     754    +4     
=====================================
+ Hits        2955    2970   +15     
- Misses       558     560    +2     
- Partials     193     194    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

JacobHayes · 2023-10-11T16:50:16Z

I'd still like to add a test run that overrides the poetry install in CI to use pydantic v1 to make sure that does (and continues to) work.

Do you all have any suggestions? Perhaps adding a simplified test-pydantic-v1 job to .github/workflows/cicd.yaml that overrides the installed version (that doesn't use a matrix, upload coverage, etc)?

flaviuvadan · 2023-10-12T01:12:56Z

I'd still like to add a test run that overrides the poetry install in CI to use pydantic v1 to make sure that does (and continues to) work.

Do you all have any suggestions? Perhaps adding a simplified test-pydantic-v1 job to .github/workflows/cicd.yaml that overrides the installed version (that doesn't use a matrix, upload coverage, etc)?

This sounds great to me. Could it be included in the matrix build maybe?

JacobHayes · 2023-10-12T01:31:02Z

Are you thinking a new pydantic_major_version: [1, 2] arg and just always do something like pip install 'pydantic==$pydantic_major_version (w/ whatever tweaks pip needs to "take latest for that major version") after the poetry install? It does add a lot of new jobs but I guess the parallelism takes care of that haha.

One thing I'm not certain about is whether mypy will work across both versions (which is one reason for the test in the first place 😁) - hopefully we don't need to tweak the job any if there are issues there.

flaviuvadan · 2023-10-12T14:45:38Z

It does add a lot of new jobs but I guess the parallelism takes care of that haha

Yea, that's what I was thinking! And also this would be temporary. Perhaps we can remove it in a few months.

One thing I'm not certain about is whether mypy will work across both versions (which is one reason for the test in the first place 😁) - hopefully we don't need to tweak the job any if there are issues there.

Hopeful that we don't have to change it :)

flaviuvadan · 2023-10-12T14:46:30Z

@samj1912 and @elliotgunton this PR + approach LGTM. Any chance I can trouble you to review? Also weigh in on the Q of pydantic V1 vs V2 testing

JacobHayes · 2023-10-24T20:36:16Z

Going to work on adding pydantic versions to build matrix here in a bit

.github/workflows/cicd.yaml

flaviuvadan · 2023-10-29T03:29:03Z

@elliotgunton and @samj1912 nudge to review this. Again, LGTM, and I am a fan of the second job approach to verify pydantic v1 backwards compatibility

flaviuvadan · 2023-10-30T22:58:19Z

@JacobHayes what do you think about appending

[mypy-pydantic.*]
ignore_missing_imports = True

to mypy.ini prior to running pydantic v1 tests in CI?

elliotgunton

Awesome stuff @JacobHayes! 🚀

On this:

add a couple more tests and try to support serializing v1 and v2 user-defined models (not just the v1 models from either version - I think this should be possible).

Have you been able to add v2 models? I think a couple of tests using v2 models mirroring the callable_script.py example would suffice!

src/hera/shared/_pydantic.py

.github/workflows/cicd.yaml

JacobHayes · 2023-12-01T16:15:59Z

Thanks for the suggestions, circling back to this today. 🚀

I think we should be able to get v1 and v2 user models serialized, but I think it'd be best (or at least easiest 😁) for the Hera internal models to be "all v1" or "all v2" until we move to v2 only.

JacobHayes · 2023-12-03T03:23:06Z

rebased, but will follow up on the rest tomorrow

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Signed-off-by: Matt Rose <mattyrose@canva.com>

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

sambhav · 2023-12-07T23:44:19Z

examples/workflows/scripts/callable_script.py

@@ -89,7 +91,7 @@ def function_kebab_object(annotated_input_value: Annotated[Input, Parameter(name
 with Workflow(name="my-workflow") as w:
    with Steps(name="my-steps") as s:
        my_function(arguments={"input": Input(a=2, b="bar", c=42)})
-        str_function(arguments={"input": Input(a=2, b="bar", c=42).json()})
+        str_function(arguments={"input": serialize(Input(a=2, b="bar", c=42))})


This needs to be changed to serialize function in hera, which correctly handles dumping both v1 and v2 objects.

sambhav · 2023-12-07T23:45:55Z

tests/test_runner.py

-        pytest.param(
-            "tests.script_runner.parameter_inputs:str_subclass_parameter_expects_jsonstr_dict",
-            [{"name": "my_json_str", "value": json.dumps({"my": "dict"})}],
-            {"my": "dict"},
-            id="str-subclass-json-param-as-dict",
-        ),
-        pytest.param(
-            "tests.script_runner.parameter_inputs:str_subclass_annotated_parameter_expects_jsonstr_dict",
-            [{"name": "my_json_str", "value": json.dumps({"my": "dict"})}],
-            {"my": "dict"},
-            id="str-subclass-json-annotated-param-as-dict",
-        ),


Had to remove these tests as MyStr is considered an arbitrary type in pydantic v2 and we don't have arbitrary types enabled even for v1. Made more sense to just get rid of this example (which we had added of our own volition in a recent PR by @elliotgunton )

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

elliotgunton

Nothing blocking, looking good 🚀

src/hera/workflows/runner.py

src/hera/shared/serialization.py

src/hera/workflows/runner.py

tests/test_examples.py

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

JacobHayes · 2024-04-03T15:53:27Z

This would have (or could 😁) simplify mypy things across versions: pydantic/pydantic#9042

JacobHayes force-pushed the pydantic-v1-v2 branch 2 times, most recently from 39388ea to 8ddabf3 Compare October 11, 2023 15:53

flaviuvadan added type:enhancement A general enhancement semver:minor A change requiring a minor version bump labels Oct 11, 2023

JacobHayes force-pushed the pydantic-v1-v2 branch 2 times, most recently from ed785c6 to 08bcfec Compare October 11, 2023 16:46

JacobHayes marked this pull request as ready for review October 11, 2023 16:46

JacobHayes requested review from flaviuvadan, sambhav and elliotgunton as code owners October 11, 2023 16:46

JacobHayes force-pushed the pydantic-v1-v2 branch from 08bcfec to bc5ac35 Compare October 24, 2023 20:31

JacobHayes commented Oct 25, 2023

View reviewed changes

.github/workflows/cicd.yaml Show resolved Hide resolved

elliotgunton reviewed Nov 23, 2023

View reviewed changes

src/hera/shared/_pydantic.py Outdated Show resolved Hide resolved

.github/workflows/cicd.yaml Show resolved Hide resolved

JacobHayes force-pushed the pydantic-v1-v2 branch from ee2480d to fcdd964 Compare December 3, 2023 03:21

JacobHayes added 2 commits December 7, 2023 21:40

Update deps to support pydantic 2

86487bb

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Configure datamodel-codegen for pydantic v1 BaseModel syntax

281dfdd

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

sambhav force-pushed the pydantic-v1-v2 branch 7 times, most recently from 36a504f to 70e7a9e Compare December 7, 2023 23:40

matty-rose and others added 6 commits December 7, 2023 23:41

Fix datamodel-codegen dir output after update

6009d93

Signed-off-by: Matt Rose <mattyrose@canva.com>

Update pydantic imports to use hera.shared._pydantic

3764d62

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Update doc examples to use pydantic v1/v2 compatible imports

c63c044

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Fix user-defined model serialization

14cc6de

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Add pydantic v1 tests in CI

9652315

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>

Add support for pydantic v2 in hera runner

71a4557

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

sambhav force-pushed the pydantic-v1-v2 branch from 70e7a9e to 71a4557 Compare December 7, 2023 23:41

sambhav reviewed Dec 7, 2023

View reviewed changes

sambhav approved these changes Dec 7, 2023

View reviewed changes

Add coverage from pydantic v1 as well

de41684

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

sambhav enabled auto-merge (squash) December 7, 2023 23:53

sambhav disabled auto-merge December 7, 2023 23:53

elliotgunton reviewed Dec 8, 2023

View reviewed changes

sambhav force-pushed the pydantic-v1-v2 branch 2 times, most recently from 685aaa5 to 6c22fb1 Compare December 8, 2023 12:13

sambhav enabled auto-merge (squash) December 8, 2023 12:13

Address comments about str types and python version

2b8ed33

Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>

sambhav force-pushed the pydantic-v1-v2 branch from 6c22fb1 to 2b8ed33 Compare December 8, 2023 12:15

sambhav merged commit f0279c8 into argoproj-labs:main Dec 8, 2023
19 checks passed

JacobHayes deleted the pydantic-v1-v2 branch December 15, 2023 01:24

elliotgunton mentioned this pull request Mar 5, 2024

Pydantic should default to v2, or hera should allow it to be configured #984

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for pydantic v2 #795

Add support for pydantic v2 #795

JacobHayes commented Oct 1, 2023 •

edited by sambhav

Loading

JacobHayes commented Oct 1, 2023 •

edited

Loading

matty-rose commented Oct 10, 2023

JacobHayes commented Oct 11, 2023

codecov bot commented Oct 11, 2023 •

edited

Loading

JacobHayes commented Oct 11, 2023

flaviuvadan commented Oct 12, 2023

JacobHayes commented Oct 12, 2023

flaviuvadan commented Oct 12, 2023

flaviuvadan commented Oct 12, 2023

JacobHayes commented Oct 24, 2023

flaviuvadan commented Oct 29, 2023

flaviuvadan commented Oct 30, 2023

elliotgunton left a comment

JacobHayes commented Dec 1, 2023

JacobHayes commented Dec 3, 2023

sambhav Dec 7, 2023

sambhav Dec 7, 2023

elliotgunton left a comment

JacobHayes commented Apr 3, 2024

Add support for pydantic v2 #795

Add support for pydantic v2 #795

Conversation

JacobHayes commented Oct 1, 2023 • edited by sambhav Loading

JacobHayes commented Oct 1, 2023 • edited Loading

matty-rose commented Oct 10, 2023

JacobHayes commented Oct 11, 2023

codecov bot commented Oct 11, 2023 • edited Loading

Codecov Report

JacobHayes commented Oct 11, 2023

flaviuvadan commented Oct 12, 2023

JacobHayes commented Oct 12, 2023

flaviuvadan commented Oct 12, 2023

flaviuvadan commented Oct 12, 2023

JacobHayes commented Oct 24, 2023

flaviuvadan commented Oct 29, 2023

flaviuvadan commented Oct 30, 2023

elliotgunton left a comment

Choose a reason for hiding this comment

JacobHayes commented Dec 1, 2023

JacobHayes commented Dec 3, 2023

sambhav Dec 7, 2023

Choose a reason for hiding this comment

sambhav Dec 7, 2023

Choose a reason for hiding this comment

elliotgunton left a comment

Choose a reason for hiding this comment

JacobHayes commented Apr 3, 2024

JacobHayes commented Oct 1, 2023 •

edited by sambhav

Loading

JacobHayes commented Oct 1, 2023 •

edited

Loading

codecov bot commented Oct 11, 2023 •

edited

Loading