typeddjango / django-stubs

PEP-484 stubs for Django
MIT License
1.62k stars 452 forks source link

Using django-stubs with django-multitenant #1606

Open Mapiarz opened 1 year ago

Mapiarz commented 1 year ago

Hi!

I'm able to integrate with django-stubs no problem. It does work as intended overall. In my project though, I'm using django-multitenant which adds a layer of complexity. Almost ALL of my relations are modeled using TenantForeignKey. Similarly, almost all of my types are using TenantManager instead of the built-in Django Manager. This effectively breaks django-types and I'm not getting the correct type hints anywhere in my project.

See example below:

class MyBaseClass(Model):
    objects = TenantManager()

class MyFooClass(MyBaseClass):
    ...

class MyBarClass(MyBaseClass):
    my_foo = TenantForeignKey[MyFooClass](MyFooClass, on_delete=CASCADE)

Almost all types in my project are derived from MyBaseClass. All Queryset operations such as MyFooClass.objects.get() will return Any. Foreign relations are not annotated correctly either.

Please advise. What's the path of least resistance to get this working? I appreciate all the help I can get.

sobolevn commented 1 year ago

Yes, because we hardcode ForeingKey in our plugin, so it is not that easy to change it to something else.

As a workaround you can do:

if TYPE_CHECKING:
   TenantForeignKey: TypeAlias = ForeighKey
else:
    # Import real implementation
Mapiarz commented 1 year ago

Thanks @sobolevn.

I tried this for one of my classes and unfortunately that alone doesn't seem to cut it. The related fields are Any. Is there something still missing? Can I debug this somehow?

flaeppe commented 1 year ago

My guess is that it could be incorrect due to the mypy plugin code relying on the runtime type of fields/models/managers.

I suppose one could attempt to declare some similar inheritance from ForeignKey in django-stubs tests to rule out if that's an issue for the plugin or not.

sobolevn commented 1 year ago

Good idea, please feel free to send a PR

Mapiarz commented 1 year ago

I was able to integrate django-multitenant by creating stubs for their models like this:

from typing import (
    TypeVar,
)

from django.db import models
from django_multitenant import mixins

_M = TypeVar("_M", bound=models.Model)

class TenantManager(models.Manager[_M]): ...
class TenantModel(mixins.TenantModelMixin, models.Model): ...

and

from typing import (
    Optional,
    TypeVar,
)

from django.db import models

_M = TypeVar("_M", bound=Optional[models.Model])

class TenantForeignKey(models.ForeignKey[_M]): ...
class TenantOneToOneField(models.OneToOneField[_M]): ...

The boundary is not exactly right as it should be intersection between django's Model and TenantModelMixin. This will allow somebody to put a tenant manager on a model without tenant mixin or do a tenant relation to a model without tenant mixin. This is a limitation I'm okay with.

Perhaps one could try to declare a type that combines the Model and TenantModelMixin and use that as a boundary but I couldn't get it to work with my non-trivial inheritance structure. Either I was doing something wrong or type checkers don't handle that case too well.

Either way, I'm content with my approach. Perhaps one day somebody will be bothered by this enough (and will have the right skillset, which I don't) to work on a general solution :)

Thanks! Feel free to close this issue if you deem it so.