Pulp

The proposed solution of calling save_related() after the batch has been bulk created only supports one-to-many related models (with FK to Content).

Changing this to:

def _create_batch(batch)
    <move batch create code block here>

plugin writer would override as:

def _create_batch(batch):
     <create models where Content has FK to>
    super()._create_batch(batch)
    <create models with FK to Content>

This is step toward functional decomposition of the call and supports both one-to-many and many-to-one.

That said, having Content with FK to other models could introduce orphan clean up problems and can't think of a good use case for it. But, thinking the API should be as flexible as possible to avoid being short sighted.

In either case the new method should be protected (leading _).

Thoughts?

I can't think of a specific situation, but I can imagine someone wanting to do some saving within the transaction prior to the content unit being saved. I think the hooks are simpler for this specific pattern. The hooks would be:

class CustomContentUnitSaver(ContentUnitSaver):

    def save_before(batch):
        """
        """

    def save_safter(batch):
        """
        """

I think the hooks are less hazard prone because they don't have to call super(). Also it encourages them to separate their before/after code into distinct functions.

What do you think?

Calling super when overriding a method is common (recommended) practice and I'd be surprised if it posed a hazard. I prefer the approach I suggested mainly because it's a pattern I'm used to seeing. But, what you're proposing in #3953-4 will meet our needs as well.