pydantic / pydantic-core

Core validation logic for pydantic written in rust
MIT License
1.44k stars 247 forks source link

Rust stacktraces and panics with validate_assignment and custom fields #1516

Closed devkral closed 2 weeks ago

devkral commented 3 weeks ago

Environment:

In the edgy project we customize the fields and models heavily. We inherit from FieldInfo and provide our own information.

Bug:

When turning on validate_assignment suddenly rust stack traces appear for some tests. In particular when we modify the fields so they aren't required anymore for the copy.

How to reproduce:

Run the test suite of:

https://github.com/dymmond/edgy/tree/devkral/potential_pydantic_bugs

with hatch test

It seemingly happens when assigning an attribute.

devkral commented 3 weeks ago

I couldn't get a traceback but could extract this: thread '' panicked at src/validators/model_fields.rs:422:85:

devkral commented 3 weeks ago

I could trace it back to the __dict__ overwrite. I think the correct behavior would be to fail gracefully instead of panicking.

davidhewitt commented 3 weeks ago

Thanks for the report. I tried to run hatch test but there's some kind of connection error in all the tests. Are you able to share a minimal reproduction? It would be very helpful compared to me trying to pick apart the unique features of your codebase.

devkral commented 3 weeks ago

you run out of file descriptors. Strangely this happens, maybe the tests run too fast and the file descriptors are not returned fast enough. I could it trace it back to the PKField and one other to a __dict__ overwrite I fixed. I will provide a new branch soon.

devkral commented 3 weeks ago

https://github.com/dymmond/edgy/tree/devkral/potential_pydantic_bugs2

The test command is hatch test tests/reflection/test_table_reflection_special_fields.py

Edit: the link is now correct. This doesn't overwrite the __dict__ anymore

devkral commented 3 weeks ago

a quick fix for running the complete test suite is running ulimit -n 10000 before running in the same context the test-suite

davidhewitt commented 3 weeks ago

That command is still not working for me:

===================================================================================== short test summary info ======================================================================================
ERROR tests/reflection/test_table_reflection_special_fields.py::test_can_reflect_correct_columns - OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 5432, 0, 0), [Errno 111] Connect call failed ('127.0.0.1', 5432)
ERROR tests/reflection/test_table_reflection_special_fields.py::test_create_correct_inspect_db - OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 5432, 0, 0), [Errno 111] Connect call failed ('127.0.0.1', 5432)
ERROR tests/reflection/test_table_reflection_special_fields.py::test_create_correct_inspect_db_with_full_info_avail - OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 5432, 0, 0), [Errno 111] Connect call failed ('127.0.0.1', 5432)
ERROR tests/reflection/test_table_reflection_special_fields.py::test_can_read_update_fields - OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 5432, 0, 0), [Errno 111] Connect call failed ('127.0.0.1', 5432)
======================================================================================== 4 errors in 10.25s ========================================================================================
devkral commented 3 weeks ago

you have to start the docker services with: docker compose up -d first.

devkral commented 3 weeks ago

We forgot to write it in the contributing section, sry

devkral commented 3 weeks ago

I just added somewhere a comment which is clearly not enough.

davidhewitt commented 3 weeks ago

Ok, I managed to reproduce and have opened #1532 to fix the panic. Thanks.