Clarification about on_delete in Django 2.0 about django-modelcluster HOT 8 CLOSED

Thanks @lb- - on_delete is indeed required, as per ForeignKey. Have updated the examples in the readme now.

from django-modelcluster.

I suggest we reopen this issue.

Updating large projects to Wagtail 2.0 is pretty painful only because of this. The module rename is very easy using the script, but we have to manually add the on_delete=CASCADE attribute on each ParentalKey.

Which brings me to this question: is there any case where we don’t want ParentalKey to be on_delete=CASCADE?
The whole purpose of ParentalKey is to create models that are “contained” in a main model. When we delete a main object like a Wagtail page, we always want the data “contained” in that main object to be deleted as well. So ParentalKey should always have CASCADE behaviour as far as I can tell.

And therefore, what’s the point of having to manually specify it? We could force on_delete=CASCADE on ParentalKeys and don’t even accept any optional on_delete argument since it wouldn’t make sense.

Side note:

When I worked on some advanced closed-source cross-models importer, I always took the assumption that on_delete=CASCADE meant that the model is “contained” in another model. In a correctly designed database, that’s what CASCADE means. All that to explain that not only ParentalKey ⇒ CASCADE, but also CASCADE ⇒ ParentalKey in a correctly designed database.

from django-modelcluster.

on_delete=models.CASCADE giving errors, requires arguments

from django-modelcluster.

TypeError: CASCADE() missing 4 required positional arguments: 'collector', 'field', 'sub_objs', and 'using'

from django-modelcluster.

@setriakorm You shouldn't put () after CASCADE. If that's not the issue, please provide specific steps to reproduce this error.

from django-modelcluster.

from django-modelcluster.

I think the best way would be allowing users to set on_delete to a default value for all cases in settings.py. If not set, then it would be a required argument; if set, then use that default value for all cases.

from django-modelcluster.

@morningmorningwind I disagree... we want this package to be as useful and friendly as possible in its default configuration. on_delete=CASCADE is the right choice for ParentalKey in every scenario we can imagine, so it makes sense for it to be the default. Moving it into a configuration option is a poor way to serve users - it means that only the users who take time to read through all the configuration options will benefit from this.

from django-modelcluster.