Opened 6 years ago

Closed 3 years ago

#15107 closed Cleanup/optimization (fixed)

Convert core commands to use self.std(out|err) instead of sys.std(out|err)/print

Reported by: mmcnickle Owned by: mmcnickle
Component: Core (Management commands) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


Some core management commands are using raw "print" statements and calls to sys.stdout/sys.stderr instead of self.stdout/self.stderr. This causes an inconsistency between commands when people specify their own buffers when using

out = StringIO()
err = StringIO()
call_command('command', stdout=out, stderr=err)

Attachments (3)

use-self.diff (9.8 KB) - added by adamv 6 years ago.
Use self.stderr|out instead of print or sys.
15107-2.diff (71.0 KB) - added by claudep 5 years ago.
Commands output through logging
15107-3.diff (70.4 KB) - added by claudep 4 years ago.
Updated to current trunk

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

For background -- the transition to self.stdout/stderr was done to ensure easy testing, and as a side effect, to allow Django commands to be used as a service. We've converted those commands that needed to be converted for the purpose of Django's own testing; the remainder is left as a work in progress.

Changed 6 years ago by adamv

Use self.stderr|out instead of print or sys.

comment:2 Changed 6 years ago by adamv

  • Has patch set

comment:3 Changed 6 years ago by jezdez

I would rather like to see the logging module be used here, if possible as we've seen a tremendous slowdown when we've ported the staticfiles app to django and removed the logging from commands.

Version 0, edited 6 years ago by jezdez (next)

comment:4 Changed 6 years ago by adamv

  • Patch needs improvement set

@jezdez If you point me to an example where logging is used "the right way", I'll try to work up a patch.

comment:5 Changed 6 years ago by jezdez

In django-staticfiles we had an own management command specific logger mixin, but I admit that's a bit cumbersome given our new logging API in 1.3. I'd like to propose to add an additional logger (next to the 'django', 'django.request' and 'django.db.backends' loggers (, e.g. 'django.managment.commands' or 'django.commands'.

Last edited 6 years ago by jezdez (previous) (diff)

comment:6 Changed 5 years ago by lrekucki

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:7 Changed 5 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Changed 5 years ago by claudep

Commands output through logging

comment:8 Changed 5 years ago by claudep

  • Easy pickings unset
  • Patch needs improvement unset
  • UI/UX unset

I worked on a refactor of commands output using logging infrastructure, adding new and BaseCommand.error() proxy methods, and deprecating self.stderr and self.stdout. Hopefully this is in the right direction, as of jezdez comment:5.

comment:9 Changed 4 years ago by claudep

Just a note to any potential reviewer: as this patch is quite extensive, I will keep a local branch updated with current trunk. However, I will not spam this ticket each time I rebase my branch. So just ping me if/when you are interested to review it (hopefully during the 1.5 timeframe!)

Changed 4 years ago by claudep

Updated to current trunk

comment:10 Changed 4 years ago by Mike <leachim@…>

If BaseCommand has .info() and .error(), it might be nice to be complete and include .debug(), .warning() and .critical(), that also get output to the stdout/stderr passed to the command.

Then the verbosity argument could potentially be updated to also take strings, so you could for example do -v W to only have output at warning level or higher.

comment:11 Changed 4 years ago by claudep

  • Patch needs improvement set

Thanks Mike, that's an interesting comment. I'd like to push patch for #18325 first, then rewrite this one with your idea in mind.

comment:12 Changed 3 years ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

Let's close this one in favour of #21429

Note: See TracTickets for help on using tickets.
Back to Top