Conversation
colesmj
left a comment
There was a problem hiding this comment.
Thank you for submitting this refactoring of the pytm base code; it was long overdue. The change to pydantic instead of the legacy var mechanism will make for cleaner code and also create some new possibilities for extending pytm.
No threats needed to change to support this refactor (e.g. the condition evals were all "safe" with the checks you added)?
Thanks for the review and the comments! There are a bunch of changes I'd like to make but I wanted to keep it invisible to most end users. Maybe with more drastic changes this might become part of a 2.0 release? No threats needed to change for now. I did think about moving the threat library to python code, but there was not a lot to gain other than using enums and classes (to save string matching). |
|
I've made some changes to add docstrings to help address #294 |
colesmj
left a comment
There was a problem hiding this comment.
Good changes, beneficial to all.
|
I'm just doing some final clean ups and adding a bit of test coverage. I'll try push the last changes later today. Thank you for the reviews :D |
Add additional test coverage
|
This should be all ready to go now |
Replaces the custom var descriptor system with Pydantic v2 BaseModel. This simplifies validation and makes it easier to extend. The public API is unchanged, existing threat model scripts should work without modification.
Changes:
Bug fixes:
Added Tests:
Added tests/test_pydantic_models.py covering the new validation and coercion behaviour (DataSet, field validators, Threat legacy field mapping). All existing tests pass.