Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31035 closed New feature (wontfix)

Promote BaseCommand.run_from_argv to a documented method

Reported by: Roman Odaisky Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It is sometimes useful to use django-admin commands without argparse. The most obvious reason for that would be to forward the arguments to another CLI app that parses its own arguments. For example:

class Command(BaseCommand):
    def run_from_argv(self, argv):
        celery.bin.celery.main([argv[0], *argv[2:]])  # can now access django.conf.settings

Please update the documentation to make overriding run_from_argv a supported way of implementing a management command.

Change History (4)

comment:1 by Carlton Gibson, 4 years ago

Resolution: wontfix
Status: newclosed

Hi Roman, thanks for the report.

I don't think this is the way forward here. run_from_argv() has more to do than just invoke the subcommand here. It's mean to catch any CommandErrors, handle the output forwarding, etc. Override it here, you loose all that.

I'd still point folks to overriding handle() as per the docs. In this case just accessing sys.argv if that's what you need. That way we don't need multiple entry points and you can catch errors, raising the expected CommandError appropriately.

in reply to:  1 comment:2 by Roman Odaisky, 4 years ago

Replying to Carlton Gibson:

I'd still point folks to overriding handle() as per the docs. In this case just accessing sys.argv if that's what you need.

That was the first thing I tried, and unfortunately it doesn’t work because the parser complains about unknown arguments. You can’t opt out of argparse with the current management command machinery.

That way we don't need multiple entry points and you can catch errors, raising the expected CommandError appropriately.

In the use case of forwarding the arguments to another CLI, this isn’t necessary at all.

Maybe another overridable method should be introduced between run_from_argv() and handle()?

comment:3 by Carlton Gibson, 4 years ago

Maybe another overridable method should be introduced between run_from_argv() and handle()?

Yes... what's needed I guess is a way to tell the parser to use `parse_known_args()`
(a self.parse_args() hook I suppose).

Is this something you'd want to work on?

in reply to:  3 comment:4 by Roman Odaisky, 4 years ago

Replying to Carlton Gibson:

Maybe another overridable method should be introduced between run_from_argv() and handle()?

Yes... what's needed I guess is a way to tell the parser to use `parse_known_args()`
(a self.parse_args() hook I suppose).

Is this something you'd want to work on?

The extra method would be trivial, see below—commands can override execute_argv() while any important cleanup code can still be added to run_from_argv (which will continue being an undocumented implementation detail). Using parse_known_args also wouldn’t be very hard, except if the 3rd party CLI to which you’re forwarding the arguments accepts, say, --verbosity=4, Django won’t pass that through.

diff --git a/django/core/management/base.py b/django/core/management/base.py
index 0376d67662..619e9503e3 100644
--- a/django/core/management/base.py
+++ b/django/core/management/base.py
@@ -317,6 +317,17 @@ class BaseCommand:
         ``Exception`` is not ``CommandError``, raise it.
         """
         self._called_from_command_line = True
+        try:
+            self.execute_argv(argv)
+        finally:
+            try:
+                connections.close_all()
+            except ImproperlyConfigured:
+                # Ignore if connections aren't setup at this point (e.g. no
+                # configured settings).
+                pass
+
+    def execute_argv(self, argv):
         parser = self.create_parser(argv[0], argv[1])
 
         options = parser.parse_args(argv[2:])
@@ -324,10 +335,11 @@ class BaseCommand:
         # Move positional args out of options to mimic legacy optparse
         args = cmd_options.pop('args', ())
         handle_default_options(options)
+
         try:
             self.execute(*args, **cmd_options)
-        except Exception as e:
-            if options.traceback or not isinstance(e, CommandError):
+        except CommandError as e:
+            if options.traceback:
                 raise
 
             # SystemCheckError takes care of its own formatting.
@@ -336,13 +348,6 @@ class BaseCommand:
             else:
                 self.stderr.write('%s: %s' % (e.__class__.__name__, e))
             sys.exit(1)
-        finally:
-            try:
-                connections.close_all()
-            except ImproperlyConfigured:
-                # Ignore if connections aren't setup at this point (e.g. no
-                # configured settings).
-                pass
 
     def execute(self, *args, **options):
         """
Note: See TracTickets for help on using tickets.
Back to Top