SAWarning on EmailAddress

[Mogost]

/lib/python3.8/site-packages/aldjemy/orm.py:160: 
SAWarning: This declarative base already contains a class with the same class name and module name 
as aldjemy.orm.EmailAddress, and will be replaced in the string-lookup table.
mapper_registry.map_imperatively(sa_model, local_table=table, properties=attrs)

Probably after 150353d

django 3.2
aldjemy 2.3

[ryanhiebert]

@ticosax any thoughts? @Mogost are you able to give a minimal reproduction?

[ticosax]

Thanks @Mogost, we haven't experienced this regression with our own project.
We would need more information to reproduce and hopefully get a fix for it.

[Mogost]

@ticosax Apparently this is due to the fact that in my project there is a model class named EmailAddress
I quickly tried renaming and the error went away. But even so, from the point of view of django, name is correct. And I'm not sure that rename is the right decision.

class EmailAddress(models.Model):
    created = models.DateTimeField(default=timezone.now, verbose_name=_('Created'))
    user = models.ForeignKey(Employee, models.CASCADE, related_name='email_addresses', verbose_name=_('User'))
    domain = models.CharField(max_length=254, verbose_name=_('Domain'))
    email = models.EmailField(unique=True, verbose_name=_('Email'))
    primary = models.BooleanField(default=False, verbose_name=_('Primary'))

[ticosax]

I'm sorry, that's not enough information for me.
Did you try to write a reproducible test case ?

[tim-schilling]

I'm pretty sure this occurs when you have different apps with the same model name. For example, if you have an app that defines a separate model named Group, it will throw this error message with django out of the box due to django.contrib.auth.Group.

[tim-schilling]

I'm not entirely sure how it gets triggered. It may be enough to run python manage.py to view the management commands. Otherwise, running the following somewhere to generate SQLAlchemy models should do the trick if you have warnings being monitored:

from sqlalchemy import MetaData
from aldjemy.orm import construct_models
construct_models(MetaData(schema="public"))

Assuming your postgres database's schema is public.

[ryanhiebert]

Assuming your postgres database's schema is public.

I don't think Aldjemy introspects the database, so I expect you should be able to produce the same result even if you use the incorrect schema name.

[tim-schilling]

I suspect the call to generate_tables(metadata) in construct_models is doing the introspection on the db.

[tim-schilling]

Yeah, I was able to confirm this. If you have models with the same name in two different apps, calling construct_models will cause this warning to be raised when mapper_registry.map_imperatively(sa_model, local_table=table, properties=attrs) is executed where the second sa_model is used. This is likely due to using model._meta.object_name for the dynamic class name here.

This is probably a bit difficult to resolve since it's a core difference between the Django ORM and SQLAlchemy. The core question is how do you generate the SQLAlchemy models for the following models?

auth.Group
myapp.Group

One option would be to return the sqlalchemy models with the table names rather than the Django object names. That way you know they'd be unique. Another would be to return them as their app model name: f"{app.name}_{model._meta.object_name}". Both are unintuitive from the Django side.

[ryanhiebert]

I'd be surprised to learn that SQLAlchemy assumes that there will never be a model name conflict, so that leads me to think that whatever property from SQLAlchemy this is being keyed off of is being misused by Aldjemy. I'd suspect that whatever field that is should likely be based on the table name.

[tim-schilling]

You're correct. The error does indicate it's attempting to use the module. I couldn't quickly find where that's being set. Given the example below says __main__ and I was running it in a shell, it's could be where the registry is created.

This declarative base already contains a class with the same class name and module name as main.Group

[ryanhiebert]

Oh, that's interesting. I wonder if we can modify the classname we generate for the Aldjemy model to something better. That might be a challenge for some of the auto-generated models for many-to-many through tables. Do you have an example project you can put on GitHub for me to reproduce?

[tim-schilling]

Nope, but if you add a model named Group to your test_project in the repo, it should reproduce the issue.

[ryanhiebert]

Here's the warning I got:

[...]/aldjemy/orm.py:165: SAWarning: This declarative base already contains a class with the same class name and module name as aldjemy.orm.Group, and will be replaced in the string-lookup table.

[tim-schilling]

Yep, that's the one.

[Mogost]

Yes exactly. I have only one EmailAddress model in my project. But I have the django-allauth app installed, which also has an EmailAddress model.
I checked out the fix and it does remove this warning.

[tim-schilling]

Thanks for the quick change @ryanhiebert!

[ryanhiebert]

You bet! Thanks, @tim-schilling , for making it easy to reproduce and giving me some really good hints!