Calling "manage.py reset <app>" on an app that has no models causes crash

[gav@…]

python manage.py --noinput reset <app>, where the specified app does not have any classes that extend models.Model, results in this error:

Traceback (most recent call last):
  File "manage.py", line 12, in <module>
    execute_manager(settings)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/core/management.py", line 1744, in execute_manager
    execute_from_command_line(action_mapping, argv)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/core/management.py", line 1701, in execute_from_command_line
    output = action_mapping[action](mod, options.interactive)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/core/management.py", line 699, in reset
    sql_list = get_sql_reset(app)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/core/management.py", line 384, in get_sql_reset
    return get_sql_delete(app) + get_sql_all(app)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/core/management.py", line 370, in get_sql_delete
    app_label = app_models[0]._meta.app_label
IndexError: list index out of range

app_label on line 370 in django/core/management.py doesn't seem to be used anywhere, it's at the end of the method. Removing that line prevents the crash and appears not to have any side effects.

Even though it doesn't necessarily make sense to call reset on an app with no models, it is a hindrance to have this crash occur if you are, for instance, writing scripts to reset all your apps (since a reset_all isn't provided in django.core.management), or doing anything else where you might reset multiple apps at once without knowing their contents.

[Chris Beaven]

Agreed. Care to attach the simple patch?

[gav@…]

Patch against r5788 that resolves reset crash on apps with no models.

[gav@…]

Patch now attached.

[Adrian Holovaty]

I agree that we should fix this, but what an odd patch...Removing that line seems like it would have strange side effects.

Let's explore the repercussions of this change before committing the patch.

[George Vilches <gav@…>]

I've run all the unit tests, it does not create any new errors or failures.

As far as side effects, what might not be obvious from the patch is that the variable is not used in that method. In fact, here's what the applicable portion of the function looks like (the ellipsis is just because the function is rather long before it gets here):

def get_sql_delete(app):
    ...
    app_label = app_models[0]._meta.app_label 

    # Close database connection explicitly, in case this output is being piped
    # directly into a database client, to avoid locking issues.
    if cursor:
        cursor.close()
        connection.close()

    return output[::-1] # Reverse it, to deal with table dependencies.
get_sql_delete.help_doc = "Prints the DROP TABLE SQL statements for the given app name(s)."
get_sql_delete.args = APP_ARGS

That's it. That variable is never used anywhere.

As far as the question of maybe it initializes something in the model through a specialized getter, I don't see this being the case. Take a look at django/db/models/base.py, in ModelBase.new (lines 47-51):

        if getattr(new_class._meta, 'app_label', None) is None:
            # Figure out the app_label by looking one level up.
            # For 'django.contrib.sites.models', this would be 'sites'.
            model_module = sys.modules[new_class.__module__]
            new_class._meta.app_label = model_module.__name__.split('.')[-2]

So _meta.app_label is always ensured to exist and be initialized. As far as the usage of _meta, there's nothing special about the _meta dict anywhere that would take advantage of accessing this variable to do more initialization than what happens in ModelBase and Model's init, and this SQL delete management call is far past that point.

So, with all that, I think this change looks pretty safe.

[George Vilches <gav@…>]

Taking ownership of this ticket for the sprint.

[George Vilches]

Updates patch to fix resets on empty models to r6979.

[George Vilches]

Updated the patch, although it's still fundamentally the same, and still has the same crash error on an app with no models. I still haven't found any reason for this line to exist, does anyone else have an idea?

[Chris Beaven]

All I can suggest is raising it in django-dev again, gav.

[Karen Tracey]

This appears to have been fixed as a side-effect of some other change prior to 1.0. With 1.0 code, running manage.py reset on an app with no models doesn't produce an error.