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

Rust stacktraces and panics with validate_assignment and custom fields #1516

Closed
devkral opened this issue Nov 1, 2024 · 11 comments · Fixed by #1532
Closed

Rust stacktraces and panics with validate_assignment and custom fields #1516

devkral opened this issue Nov 1, 2024 · 11 comments · Fixed by #1532

Comments

@devkral
Copy link

devkral commented Nov 1, 2024

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
Copy link
Author

devkral commented Nov 2, 2024

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

@devkral
Copy link
Author

devkral commented Nov 3, 2024

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

@davidhewitt
Copy link
Contributor

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
Copy link
Author

devkral commented Nov 4, 2024

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
Copy link
Author

devkral commented Nov 4, 2024

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
Copy link
Author

devkral commented Nov 4, 2024

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

@davidhewitt
Copy link
Contributor

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
Copy link
Author

devkral commented Nov 4, 2024

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

@devkral
Copy link
Author

devkral commented Nov 4, 2024

We forgot to write it in the contributing section, sry

@devkral
Copy link
Author

devkral commented Nov 4, 2024

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

@davidhewitt
Copy link
Contributor

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

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 a pull request may close this issue.

2 participants