[Django] #21429: BaseCommand should use logging instead of custom output wrappers
Django
--------------------------------------------+------------------------------
Reporter: Nical | Owner: nobody
Type: New feature | Status: new
Component: Core (Management commands) | Version: 1.6
Severity: Normal | Keywords: command output
Triage Stage: Unreviewed | logger
Easy pickings: 0 | Has patch: 0
| UI/UX: 0
--------------------------------------------+------------------------------
Since python offers the powerful logging class,
django.core.management.base.BaseCommand should use it to output messages
instead of using custom output wrappers
(django.core.management.base.OutputWrapper).
Doing so, we could override those loggers in the settings.py file.
Instead of having two wrappers in a command and having to do:
{{{
class Command(BaseCommand):
def handle(self):
self.stdout.write('this is an info message')
self.stderr.write('this is an error message')
}}}
We could do
{{{
class Command(BaseCommand):
def handle(self):
self.output.info('this is an info message')
self.output.error('this is an error message')
# and even
self.output.warning('this is a warning message')
}}}
The style_func and ending arguments that the
django.core.management.base.OutputWrapper.write method takes could be
removed and be configured at once in a overriding custom logger.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Nical | Owner: nobody
Type: New feature | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: command output | Needs documentation: 0
logger | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* version: 1.6 => master
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I worked on that idea in #15107 (that I just closed).
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:1>
Django
Reporter: Nical | Owner:
Type: New feature | berkerpeksag
Component: Core (Management | Status: assigned
commands) | Version: master
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by berkerpeksag):
* owner: nobody => berkerpeksag
* status: new => assigned
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/3467
This PR implements an initial integration of logging module in
BaseCommand. I've also updated the documentation a bit. If this approach
is okay, I'll update all management commands and tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:2>
Django
Reporter: Nical | Owner:
Type: New feature | berkerpeksag
Component: Core (Management | Status: assigned
commands) | Version: master
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by claudep):
Do we want to keep the old `stdout/stderr` way of outputting messages in
addition to the new API? This would make `self.stdout.write/self.info` and
`self.stderr.write/self.error` redundants. Or did you intend to deprecate
`self.stdout/self.stderr`?
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:3>
Django
Reporter: Nical | Owner:
Type: New feature | berkerpeksag
Component: Core (Management | Status: assigned
commands) | Version: master
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by berkerpeksag):
> Or did you intend to deprecate self.stdout/self.stderr?
Yes, but I couldn't find a simple way to deprecate them yesterday. Today,
here is my approach:
https://github.com/berkerpeksag/django/commit/b439b472842bfd2d04c36b28d079a80fac5814bb
PR updated to address Simon's comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:4>
Django
Reporter: Nical | Owner:
| berkerpeksag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:5>
Django
Reporter: Nical | Owner:
| berkerpeksag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by sposs):
I saw that the pull request was closed on github. Are there any plans to
revive this? It would be very nice to be able to configure logging like
everywhere else...
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:6>
Django
Reporter: Nical | Owner:
| berkerpeksag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
Feel free to resume work on the patch if you like.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:7>
Django
Reporter: Nical | Owner:
| ChillarAnand
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Berker Peksag => ChillarAnand
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:8>
Django
Reporter: Nical | Owner: (none)
Type: New feature | Status: new
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by ChillarAnand):
* owner: ChillarAnand => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:9>
Django
Reporter: Nical | Owner: John Kang
Type: New feature | Status: assigned
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: (none) => John Kang
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:10>
Django
Reporter: Nical | Owner: John Kang
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by John Kang):
Hi all, going to resume work on this patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:11>
Django
Reporter: Nical | Owner: John Kang
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by John Kang):
Hi,
I was working through this ticket when I noticed that some of the command
outputs (migrations for example) output a status on the same line. Example
would be:
"migrating-xx.. OK"
Python loggers implicitly add new line characters at the end.
Should I leave these log outputs with newlines or is there a better way to
append command statuses on the same line?
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:12>
Django
Reporter: Nical | Owner: John Kang
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Forest Gregg):
> Python loggers implicitly add new line characters at the end.
> Should I leave these log outputs with newlines or is there a better way
to append command statuses on the same line?
As of Python 3.2, you can set the terminator to
[[https://docs.python.org/3/library/logging.handlers.html#streamhandler|something
other than the newline in the StreamHandler]]. You could use a special
handler for logging migrations to keep the current behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:13>
Django
Reporter: Nical | Owner: François
| Freitag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: John Kang => François Freitag
Comment:
It seems this ticket stalled. I am interested in seeing that feature in
Django, resuming work on it.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:14>
Django
Reporter: Nical | Owner: François
| Freitag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by François Freitag):
An update after about 2 months. There is still a good chunk of work left,
to clean-up commits, prepare for the transition, update the documentation,
etc. but the work is taking shape: all management commands have been
updated to use a logger, tests updated accordingly and the test suite
passes.
In my draft, there’s a main logger for all commands, `django.command`, and
a logger dedicated to report progress of commands `django.progress` that
uses `\r` as a terminator. I’m planning on splitting `django.command` to
have a logger per command, e.g. `django.command.migrate`, all with
`django.command` as their parent. That will facilitate redirecting the
output of a particular command while relying on the parent behavior
otherwise.
Notable changes:
- `CommandError` assumed the message was interpolated. A `logger_args`
keyword
argument was introduced to pass variable arguments separately
(https://docs.python.org/3.8/howto/logging.html#logging-variable-data).
- Format placeholders are generated for messages where a variable number
of
arguments is used, in order to pass the variable data separately from
the
message. For example, a message listing issues in apps was previously
logged
as `"\n".join(variables)`. To log variable data separately, the message
becomes
`"\n".join(["%s"] * len(variables))`.
Left out of scope:
Some commands directly raise the message from an exception they caught as
a `CommandError`.
Ideally, variable data would be separated from the message, so that the
logger handles the variable data separately. However, the exceptions are
generated by other modules, which do not know how the exception will be
used.. In these cases, I’m using the interpolated error message for the
`CommandError`, not logging variable data separately. This can be improved
upon at a later stage.
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:15>
Django
Reporter: Nical | Owner: François
| Freitag
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"7ca7f4495ba746279b734695a8dd137bf7ee0bab" 7ca7f449]:
{{{
#!CommitTicketReference repository=""
revision="7ca7f4495ba746279b734695a8dd137bf7ee0bab"
Refs #21429 -- Added SimpleTestCase.assertNoLogs() on Python < 3.10.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:16>
Django
Reporter: Nical | Owner: (none)
commands) |
Severity: Normal | Resolution:
Keywords: command output | Triage Stage: Accepted
logger |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: François Freitag => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/21429#comment:17>