sys.exit(3) on background job that runs makemigrations - infinite loop

autodetector.md

Introduction

Issue: sys.exit(3) on background job that runs makemigrations - infinite loop

PR: https://github.com/crowdbotics/crowdbotics-slack-app/pull/1003

This gist proposes a solution to this issue by doing the same checks Django uses to either ask further questions during migrations creation or not. Currently our background job stays in a loop because we don't provide a valid answer to this questioner. Using the flag --no-input isn't enough as there is still 3 scenarios that raise the questioner.

Calling makemigrations can call either InteractiveMigrationQuestioner or NonInteractiveMigrationQuestioner.

When we send the flag --no-input to the makemigrations command it calls the non-interactive questioner.

Source code:

if self.interactive:
    questioner = InteractiveMigrationQuestioner(specified_apps=app_labels, dry_run=self.dry_run)
else:
    questioner = NonInteractiveMigrationQuestioner(specified_apps=app_labels, dry_run=self.dry_run)

NonInteractiveMigrationQuestioner itself will return sys.exit(3) in three methods.

Source code:

class NonInteractiveMigrationQuestioner(MigrationQuestioner):

    def ask_not_null_addition(self, field_name, model_name):
        # We can't ask the user, so act like the user aborted.
        sys.exit(3)

    def ask_not_null_alteration(self, field_name, model_name):
        # We can't ask the user, so set as not provided.
        return NOT_PROVIDED

    def ask_auto_now_add_addition(self, field_name, model_name):
        # We can't ask the user, so act like the user aborted.
        sys.exit(3)

With this in mind, if we want to prevent a exit code other than 0 (successful exit) we have to avoid the call of the 3 methods.

-ask_auto_now_add_addition and ask_not_null_addition

This is from where they can be called:

time_fields = (models.DateField, models.DateTimeField, models.TimeField)
preserve_default = (
	field.null or field.has_default() or field.many_to_many or
	(field.blank and field.empty_strings_allowed) or
	(isinstance(field, time_fields) and field.auto_now)
)
if not preserve_default:
	field = field.clone()
	if isinstance(field, time_fields) and field.auto_now_add:
		field.default = self.questioner.ask_auto_now_add_addition(field_name, model_name)
	else:
		field.default = self.questioner.ask_not_null_addition(field_name, model_name)

ask_not_null_alteration

This is from where it can be called:

if (old_field.null and not new_field.null and not new_field.has_default() and
		not new_field.many_to_many):
	field = new_field.clone()
	new_default = self.questioner.ask_not_null_alteration(field_name, model_name)

This one doesn't return sys.exit(3), instead it defaults to the NOT_PROVIDED option just as we chose Ignore for now.. option. This Source Code explains why its a good idea to avoid it too, check the message:

def ask_not_null_alteration(self, field_name, model_name):
	"""Changing a NULL field to NOT NULL."""
	if not self.dry_run:
		choice = self._choice_input(
			"You are trying to change the nullable field '%s' on %s to non-nullable "
			"without a default; we can't do that (the database needs something to "
			"populate existing rows).\n"
			"Please select a fix:" % (field_name, model_name),
			[
				("Provide a one-off default now (will be set on all existing "
				  "rows with a null value for this column)"),
				("Ignore for now, and let me handle existing rows with NULL myself "
				  "(e.g. because you added a RunPython or RunSQL operation to handle "
				  "NULL values in a previous data migration)"),
				"Quit, and let me add a default in models.py",
			]
		)
		if choice == 2:
			return NOT_PROVIDED
		elif choice == 3:
			sys.exit(3)
		else:
			return self._ask_default()
	return None

TL;DR

The magic sauce:

# When creating fields
field.null or field.has_default() or field.many_to_many or (field.blank and field.empty_strings_allowed) or (isinstance(field, time_fields) and field.auto_now)

# When updating fields
old_field.null and not new_field.null and not new_field.has_default() and not new_field.many_to_many

Above is all the logic Django uses to check if the questioner should display or not. We could do the same checks and rest assured that the non-interactive makemigrations never fails.

I also suggested earlier that if the request to the model builder fails this very same checks that the API should reply with an error stating that something more is required (whatever is missing).