Closed justin-p closed 1 month ago
Thanks for the interest! I see this has lots of changes packed in a single PR. It would be better to separate each atomic suggestion in an independent PR that can be individually reviewed.
I see a lot of the changes are new CRUD functions, an older version of this project had lots of them, and I actually did put a lot of effort into refactoring it to remove all the extra complexity. :sweat_smile:
Also, the docstrings depend on the specific format, I would prefer to use what I use for FastAPI, and right now I would rather not have docstirngs in these functions yet.
Also, separating models into files can lead to complex logic to handle imports, references, cyclic imports, etc. So I prefer to have the simplest structure possible and allow people to refactor and evolve as needed, you can read more about that in the SQLModel docs.
For now I'll pass on this one, but thanks for the effort! :coffee:
This PR changes the following
Refactor/restructure folders/code and add more test coverage .
app.models
orapp.crud
via the__init__.py
to prevent circular imports in combination with TYPE_CHECKING.initial_data.py
,backend_pre_start.py
,test_pre_start.py
) in the backend to a scripts folder as additional clean up.Security improvements
recover_password
to avoid it being able to be abused for user enumeration.Misc changes
96%
.settings.API_V1_STR
@mark.order("last")
to avoid issues with other tests.SECRET_KEY
to a dummy value of 32 so tests pass if left unchanged..coveragerc
file to exclude models, tests files themselves, etc, from being include in the test coverage.pytest.ini
for more control over testing.pyright ignore
's to stop vscode from complaining.