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

Disable warnings from model_dump #1214

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

alicederyn
Copy link
Collaborator

@alicederyn alicederyn commented Sep 27, 2024

Pull Request Checklist

Description of PR
Currently, using Pydantic V2 with the Input/Output mixin types and a non-str field prints a UserWarning when the class is declared, of the form "UserWarning: Pydantic serializer warning: Expected int but got str with value '{{...}}' - serialized value may not be as expected". This is caused by using model_dump on an object constructed with all string values, i.e. one which does not pass the type-checking performed by the method, as part of class creation.

This PR just adds warnings="none" to the two uses of model_dump, to disable these warnings.

When using model_dump to access attributes in experimental template decorator code, a warning is being raised due to the strings we place in the Pydantic objects not matching the declared field types. Disable these warnings.

Signed-off-by: Alice Purcell <alicederyn@gmail.com>
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.0%. Comparing base (d553546) to head (4037c57).
Report is 154 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1214     +/-   ##
=======================================
+ Coverage   81.7%   86.0%   +4.2%     
=======================================
  Files         54      60      +6     
  Lines       4208    4040    -168     
  Branches     889     840     -49     
=======================================
+ Hits        3439    3475     +36     
+ Misses       574     392    -182     
+ Partials     195     173     -22     

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

@alicederyn alicederyn added semver:patch A change requiring a patch version bump type:bug A general bug labels Sep 27, 2024


@pytest.fixture(autouse=True)
def fail_on_user_warnings():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm in two minds about this fixture — it does catch this issue and prevent any similar issues in future, but I don't like having to suppress the one deprecation warning that Pydantic appears to have accidentally left as a UserWarning. LMK if you'd rather leave it out, it's a separate commit so easy to revert.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather not optimise early here as it might get difficult/annoying when we move Hera's core code to Pydantic V2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure thing. Removed the fixture.

@elliotgunton elliotgunton enabled auto-merge (squash) September 30, 2024 10:17
@elliotgunton elliotgunton merged commit 77fa246 into argoproj-labs:main Sep 30, 2024
20 of 33 checks passed
@alicederyn alicederyn deleted the model.dump.warnings branch September 30, 2024 10:21
@elliotgunton elliotgunton added type:task A general task and removed type:bug A general bug labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants