diff --git a/django/contrib/auth/tests/management.py b/django/contrib/auth/tests/management.py index 5d31bd70a4..81ab0aa052 100644 --- a/django/contrib/auth/tests/management.py +++ b/django/contrib/auth/tests/management.py @@ -2,6 +2,7 @@ from StringIO import StringIO from django.contrib.auth import models, management from django.contrib.auth.management.commands import changepassword +from django.core.management.base import CommandError from django.test import TestCase @@ -56,16 +57,10 @@ class ChangepasswordManagementCommandTestCase(TestCase): def test_that_max_tries_exits_1(self): """ A CommandError should be thrown by handle() if the user enters in - mismatched passwords three times. This should be caught by execute() and - converted to a SystemExit + mismatched passwords three times. """ command = changepassword.Command() command._get_pass = lambda *args: args or 'foo' - self.assertRaises( - SystemExit, - command.execute, - "joe", - stdout=self.stdout, - stderr=self.stderr - ) + with self.assertRaises(CommandError): + command.execute("joe", stdout=self.stdout, stderr=self.stderr) diff --git a/django/core/management/base.py b/django/core/management/base.py index 6e06991bf0..a204f6f0bc 100644 --- a/django/core/management/base.py +++ b/django/core/management/base.py @@ -97,8 +97,9 @@ class BaseCommand(object): output and, if the command is intended to produce a block of SQL statements, will be wrapped in ``BEGIN`` and ``COMMIT``. - 4. If ``handle()`` raised a ``CommandError``, ``execute()`` will - instead print an error message to ``stderr``. + 4. If ``handle()`` or ``execute()`` raised any exception (e.g. + ``CommandError``), ``run_from_argv()`` will instead print an error + message to ``stderr``. Thus, the ``handle()`` method is typically the starting point for subclasses; many built-in commands and command types either place @@ -210,23 +211,27 @@ class BaseCommand(object): def run_from_argv(self, argv): """ Set up any environment changes requested (e.g., Python path - and Django settings), then run this command. - + and Django settings), then run this command. If the + command raises a ``CommandError``, intercept it and print it sensibly + to stderr. """ parser = self.create_parser(argv[0], argv[1]) options, args = parser.parse_args(argv[2:]) handle_default_options(options) - self.execute(*args, **options.__dict__) + try: + self.execute(*args, **options.__dict__) + except Exception as e: + if options.traceback: + self.stderr.write(traceback.format_exc()) + self.stderr.write('%s: %s' % (e.__class__.__name__, e)) + sys.exit(1) def execute(self, *args, **options): """ Try to execute this command, performing model validation if needed (as controlled by the attribute - ``self.requires_model_validation``, except if force-skipped). If the - command raises a ``CommandError``, intercept it and print it sensibly - to stderr. + ``self.requires_model_validation``, except if force-skipped). """ - show_traceback = options.get('traceback', False) # Switch to English, because django-admin.py creates database content # like permissions, and those shouldn't contain any translations. @@ -237,18 +242,9 @@ class BaseCommand(object): self.stderr = OutputWrapper(options.get('stderr', sys.stderr), self.style.ERROR) if self.can_import_settings: - try: - from django.utils import translation - saved_lang = translation.get_language() - translation.activate('en-us') - except ImportError as e: - # If settings should be available, but aren't, - # raise the error and quit. - if show_traceback: - traceback.print_exc() - else: - self.stderr.write('Error: %s' % e) - sys.exit(1) + from django.utils import translation + saved_lang = translation.get_language() + translation.activate('en-us') try: if self.requires_model_validation and not options.get('skip_validation'): @@ -265,12 +261,6 @@ class BaseCommand(object): self.stdout.write(output) if self.output_transaction: self.stdout.write('\n' + self.style.SQL_KEYWORD("COMMIT;")) - except CommandError as e: - if show_traceback: - traceback.print_exc() - else: - self.stderr.write('Error: %s' % e) - sys.exit(1) finally: if saved_lang is not None: translation.activate(saved_lang) diff --git a/docs/howto/custom-management-commands.txt b/docs/howto/custom-management-commands.txt index 2b34d35de7..4a27bdf7a9 100644 --- a/docs/howto/custom-management-commands.txt +++ b/docs/howto/custom-management-commands.txt @@ -317,8 +317,12 @@ Exception class indicating a problem while executing a management command. If this exception is raised during the execution of a management -command, it will be caught and turned into a nicely-printed error -message to the appropriate output stream (i.e., stderr); as a -result, raising this exception (with a sensible description of the +command from a command line console, it will be caught and turned into a +nicely-printed error message to the appropriate output stream (i.e., stderr); +as a result, raising this exception (with a sensible description of the error) is the preferred way to indicate that something has gone wrong in the execution of a command. + +If a management command is called from code through +:ref:`call_command `, it's up to you to catch the exception +when needed. diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 0fec53ab6d..0d86a52670 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -75,6 +75,10 @@ Django 1.5 also includes several smaller improvements worth noting: * The generic views support OPTIONS requests. +* Management commands do not raise ``SystemExit`` any more when called by code + from :ref:`call_command `. Any exception raised by the command + (mostly :ref:`CommandError `) is propagated. + * The dumpdata management command outputs one row at a time, preventing out-of-memory errors when dumping large datasets. diff --git a/tests/modeltests/fixtures/tests.py b/tests/modeltests/fixtures/tests.py index 48a5fe7f16..478bbe9129 100644 --- a/tests/modeltests/fixtures/tests.py +++ b/tests/modeltests/fixtures/tests.py @@ -185,14 +185,14 @@ class FixtureLoadingTests(TestCase): exclude_list=['fixtures.Article', 'fixtures.Book', 'sites']) # Excluding a bogus app should throw an error - self.assertRaises(SystemExit, + self.assertRaises(management.CommandError, self._dumpdata_assert, ['fixtures', 'sites'], '', exclude_list=['foo_app']) # Excluding a bogus model should throw an error - self.assertRaises(SystemExit, + self.assertRaises(management.CommandError, self._dumpdata_assert, ['fixtures', 'sites'], '', diff --git a/tests/modeltests/user_commands/management/commands/dance.py b/tests/modeltests/user_commands/management/commands/dance.py index 4ad5579e1e..911530d223 100644 --- a/tests/modeltests/user_commands/management/commands/dance.py +++ b/tests/modeltests/user_commands/management/commands/dance.py @@ -1,6 +1,6 @@ from optparse import make_option -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError class Command(BaseCommand): @@ -8,11 +8,13 @@ class Command(BaseCommand): args = '' requires_model_validation = True - option_list =[ + option_list = BaseCommand.option_list + ( make_option("-s", "--style", default="Rock'n'Roll"), make_option("-x", "--example") - ] + ) def handle(self, *args, **options): example = options["example"] + if example == "raise": + raise CommandError() self.stdout.write("I don't feel like dancing %s." % options["style"]) diff --git a/tests/modeltests/user_commands/tests.py b/tests/modeltests/user_commands/tests.py index c1e2bf9a74..509f13f62f 100644 --- a/tests/modeltests/user_commands/tests.py +++ b/tests/modeltests/user_commands/tests.py @@ -1,3 +1,4 @@ +import sys from StringIO import StringIO from django.core import management @@ -26,4 +27,20 @@ class CommandTests(TestCase): self.assertEqual(translation.get_language(), 'fr') def test_explode(self): + """ Test that an unknown command raises CommandError """ self.assertRaises(CommandError, management.call_command, ('explode',)) + + def test_system_exit(self): + """ Exception raised in a command should raise CommandError with + call_command, but SystemExit when run from command line + """ + with self.assertRaises(CommandError): + management.call_command('dance', example="raise") + old_stderr = sys.stderr + sys.stderr = err = StringIO() + try: + with self.assertRaises(SystemExit): + management.ManagementUtility(['manage.py', 'dance', '--example=raise']).execute() + finally: + sys.stderr = old_stderr + self.assertIn("CommandError", err.getvalue())