s3rius / FastAPI-template

Feature rich robust FastAPI template.
MIT License
1.9k stars 164 forks source link

Possible Bug - `get_db_session` reference included in `{{cookiecutter.project_name}}web.gql.context.py`when using Tortoise ORM #111

Closed JSv4 closed 2 years ago

JSv4 commented 2 years ago

Awesome project! Really love what you've done here!

I ran into what I think is a bug with the project generation logic. I think it's a simple fix / issue, so I'd be happy to contribute a PR, but wanted to run this by you first.

In the context.py file of the gql package, there's an import for from {{cookiecutter.project_name}}.db.dependencies import get_db_session: https://github.com/s3rius/FastAPI-template/blob/156883798ab4ec54d97080c77a34d366b8484f95/fastapi_template/template/%7B%7Bcookiecutter.project_name%7D%7D/%7B%7Bcookiecutter.project_name%7D%7D/web/gql/context.py#L16-L18

This import does not appear to be used when using Tortoise ORM, however:

https://github.com/s3rius/FastAPI-template/blob/156883798ab4ec54d97080c77a34d366b8484f95/fastapi_template/template/%7B%7Bcookiecutter.project_name%7D%7D/%7B%7Bcookiecutter.project_name%7D%7D/web/gql/context.py#L38-L42

In fact, with Tortoise ORM, there doesn't appear to be a dependencies.py file at all, see here.

Leaving the import statement throws an error with pytest:

=========================================================================================================== ERRORS ============================================================================================================
________________________________________________________________________________________________ ERROR collecting test session ________________________________________________________________________________________________
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:975: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:671: in _load_unlocked
    ???
../../.cache/pypoetry/virtualenvs/gremlinengine--i0qMi2L-py3.8/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
    exec(co, module.__dict__)
GremlinEngine/conftest.py:19: in <module>
    from GremlinEngine.web.application import get_app
GremlinEngine/web/application.py:24: in <module>
    from GremlinEngine.web.graphql.router import gql_router
GremlinEngine/web/graphql/router.py:4: in <module>
    from GremlinEngine.web.graphql import dummy, echo, redis
GremlinEngine/web/graphql/dummy/__init__.py:3: in <module>
    from GremlinEngine.web.graphql.dummy.mutation import Mutation
GremlinEngine/web/graphql/dummy/mutation.py:5: in <module>
    from GremlinEngine.web.graphql.context import Context
GremlinEngine/web/graphql/context.py:5: in <module>
    from GremlinEngine.db.dependencies import get_db_session
E   ModuleNotFoundError: No module named 'GremlinEngine.db.dependencies'

Removing the import appears to cause no other issues and resolves the error. Seems like the if/else jinja statement in the gql.context.py template to import dependencies and get_db_session should be updated to also take into account which database is being used? This is my first in-depth foray into standalone Python ORMs (I've been a Django guy, mostly), so sorry if I'm missing something here.

s3rius commented 2 years ago

Thank you for using this project! I'm glad you like it ❤️ !

Yeah, that's truly an error. I fixed it in pools issue. Will release soon.

I'll close the issue when upcoming release is merged into master branch.

JSv4 commented 2 years ago

Awesome! Thanks for the quick response and proposed resolution!

s3rius commented 2 years ago

This issue is resolved in 3.3.8. Can you please check? If it's ok, please close the issue.

s3rius commented 2 years ago

Closing due to inactivity.

JSv4 commented 2 years ago

Closing due to inactivity.

Sorry just saw this. Thanks for the fix!