Skip to content

Ensure add_stator_indexes only run for direct descendants of StatorModel#696

Open
lullis wants to merge 1 commit intojointakahe:mainfrom
mushroomlabs:stator_indexes_fix
Open

Ensure add_stator_indexes only run for direct descendants of StatorModel#696
lullis wants to merge 1 commit intojointakahe:mainfrom
mushroomlabs:stator_indexes_fix

Conversation

@lullis
Copy link
Copy Markdown
Contributor

@lullis lullis commented Jan 30, 2024

No description provided.

@AstraLuma
Copy link
Copy Markdown
Contributor

Does this have a bug associated with it? What's the behavior you're trying to prevent?

@lullis
Copy link
Copy Markdown
Contributor Author

lullis commented Feb 6, 2024

The issue is that these indices only make sense on models that derive directly from StatorModel. If you create any other model that itself is already derived from StatorModel (e.g, Identity) then the system will fail the check due to issues with the indices that are not related to the table you are creating.

@lullis lullis force-pushed the stator_indexes_fix branch from c8dcdc9 to 021d16b Compare February 9, 2024 21:21
@AstraLuma
Copy link
Copy Markdown
Contributor

Ah, ok, yeah, if you have a concrete model inheriting from a concrete model (Multi Table Inheritance), this code will fail because the indexed columns only exist on the parent table.

This largely looks good, but I'm going to double check some things.

Comment thread stator/models.py
StatorModel, otherwise we will see system check errors.
"""
if issubclass(sender, StatorModel):
if sender.__base__ is StatorModel:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely thrilled with this--__base__ is semi-documented, and points to the first parent class.

But I'm not sure what multiple inheritance would do in the context of Django models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants