tortoise / tortoise-orm

Familiar asyncio ORM for python, built with relations in mind
https://tortoise.github.io
Apache License 2.0
4.39k stars 357 forks source link

[BUG:PEP563] tortoise.contrib.pydantic.utils.get_annotations uses incorrect global namespace #1552

Closed WizzyGeek closed 1 month ago

WizzyGeek commented 5 months ago

What Happened

from __future__ import annotations

from tortoise import fields
...

class R2Container(Model):
    problem: fields.ForeignKeyRelation[R2Problem] = ...
    ports: fields.ReverseRelation[R2Ports]
...
...
# In yet another file
Tortoise.init_models(["somewhere"], "models")
R2Container_Pydantic = pydantic_model_creator(R2Container)

Raises

Traceback (most recent call last):
  [Leaving these frames for readability]
  File "/home/WizzyGeek/code/lug/pwncore/src/pwncore/models/__init__.py", line 69, in <module>
    R2Container_Pydantic = pydantic_model_creator(R2Container)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/WizzyGeek/code/lug/pwncore/.venv/lib/python3.11/site-packages/tortoise/contrib/pydantic/creator.py", line 226, in pydantic_model_creator
    annotations = get_annotations(cls)
                  ^^^^^^^^^^^^^^^^^^^^
  File "/home/WizzyGeek/code/lug/pwncore/.venv/lib/python3.11/site-packages/tortoise/contrib/pydantic/utils.py", line 18, in get_annotations
    return typing.get_type_hints(method or cls, globalns=globalns)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/typing.py", line 2358, in get_type_hints
    value = _eval_type(value, base_globals, base_locals)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/typing.py", line 381, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/typing.py", line 891, in _evaluate
    eval(self.__forward_code__, globalns, localns),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'fields' is not defined

Proposed Solution 1

https://github.com/tortoise/tortoise-orm/blob/53376c9032800770711f7a305ae971cba038b77f/tortoise/contrib/pydantic/utils.py#L17-L18

Removal of line 17 fixes the issue since the globalns goes back to the actual namespace of the object otherwise if this line is kept the namespace (both local and global) are replaced with the app namespace meaning the code for the annotations can only access models (afaik)

This solution changes behavior but I have not seen the current behavior documented or used anywhere yet.

Proposed Solution 2

Deal with internals of typing.get_type_hints and extend the globalns with app namespace

I can create a PR depending on which solution is preferred, there are also other ways if neither of the solutions are acceptable.

Workaround Here is a DI ```python import tortoise def get_annotations(cls, method = None): return typing.get_type_hints(method or cls) tortoise.contrib.pydantic.utils.get_annotations = get_annotations tortoise.contrib.pydantic.creator.get_annotations = get_annotations ```
waketzheng commented 1 month ago

I got a similar error: name 'fields' is not defined

@abondar What's the purpose of this line: globalns = tortoise.Tortoise.apps.get(cls._meta.app, None) if cls._meta.app else None, or how to fix this issue?

WizzyGeek commented 1 month ago

it's been a while I made the issue, but per my speculation, the rationale behind that seemed to be to allow using other models in the app inside annotations without imports? (which still is a questionable design choice imo)

abondar commented 1 month ago

I am not sure why it is there - but probably it is to support string typing notation, where you can make a link to other model, like you can do in ForeignKey, just by string name. So probably it was aimed at that

I would say we can delete it, as it seems obscure and it is not design I want to promote

waketzheng commented 1 month ago

Seems that solution 1 is better. @WizzyGeek could you make a PR?