django-auto-prefetch icon indicating copy to clipboard operation
django-auto-prefetch copied to clipboard

Tweak Meta.base_manager_name check message

Open adamchainz opened this issue 2 years ago • 3 comments

adamchainz avatar Apr 06 '23 10:04 adamchainz

@seporaitis @tolomea does this seem good to you?

adamchainz avatar Apr 06 '23 10:04 adamchainz

I'm unsure if this is a good idea. The default and base manager stuff rapidly gets quite confusing for people.

In the case of auto prefetch you really want both of them and probably actually all managers to either be or inherit from the auto prefetch one.

Focusing on the base manager. I think there's two ways people end up with the wrong manager. 1: the Meta inheritance chain is broken and they haven't replicated the setting down, common. 2: they are deliberately setting their own base manager, rare.

Checking the name is going to be annoying for the case 2 people, but they are also the best equipped to disable the check.

As for the case 1 people you seem to be suggesting that we should push them to replicate the setting down, which I guess works. Personally I'm a strong believer in being thorough and consistent about Meta inheritance. But maybe prefetch doesn't need to get involved in teaching these things.

tolomea avatar Apr 06 '23 10:04 tolomea

Django does meta inheritance automatically, but some of the time it doesn't work. Proxy models and intermediate abstract models seem to lose the inheritance of the attr. Perhaps I need to do more research into why, I kinda just adjusted the check for the issue @seporaitis reported.

But also you're right, we shouldn't check for the exact attr but the correct inheritance, on all managers.

adamchainz avatar Apr 07 '23 09:04 adamchainz