Make log.err a method of LogPublisher

[chris-]

The attached patch changes log.err to be an object method of LogPublisher. The goal of this patch is to make it easier to use a custom LogPublisher without loosing the convenience of log.err for handling failures, eg. when using pythons logging instead of twisted.python.log.

Besides the patch, there is a use case example attached.

[chris-]

use case example

[therve]

Your script produces exactly the same output if you replace logger.err by log.err. The change is also an backward incompatible change.

The way you customize logging in twisted is by providing a LogObserver, not a LogPublisher. Can you explain a little bit more what's your usecase or what error you're seeing?

[chris-]

Replying to therve:

Your script produces exactly the same output if you replace logger.err by log.err. The change is also an backward incompatible change.

Can you elaborate on this? log.err is still available using the global theLogPublisher instance, except now it is a method of LogPublisher available from the module scope, much like theLogPublisher.msg is available as log.msg

The way you customize logging in twisted is by providing a LogObserver, not a LogPublisher. Can you explain a little bit more what's your usecase or what error you're seeing?

I do not want to customize twisted logging, I want to use pythons logging without loosing the convenience functions like log.err. At the moment, log.err logs to the global LogPublisher, which defeats the purpose of having multiple loggers, using eg. logging.getLogger("my-module-a") and logging.getLogger("my-module-b") with pythons logging. I could use the PythonLoggingObserver with twisted log, however this would mean that if I'd use log.err, all observers and thus both the logger in "my-module-a" and the logger in "my-module-b" would get notified on an error in module A which kind of defeats the point in having them. So from my perspective, the best way to go would be to create a logging.Logger which can also handle the twisted way of formatig err cases, as shown in the example.

[therve]

Ah, thanks for the hint, I missed the assignment. I don't really use stdlib logging, so I don't know if your story is reasonable or not.

It sounds like there should be a way to specify the publisher for this to not get too ugly. The fact that you have an object which is both a Logger and a LogPublisher, and override the observer logger, which looks like a weird API.

[exarkun]

Can you explain the purpose of this change more? It's difficult to understand what the desired outcome is (as another non-stdlib-logging module user). My initial reaction is that you should be customizing log events, not having multiple log publishers.

[chris-]

Replying to therve:

It sounds like there should be a way to specify the publisher for this to not get too ugly. The fact that you have an object which is both a Logger and a LogPublisher, and override the observer logger, which looks like a weird API.

Yes, the API in the example is not that nice, but then again, it's only an example, not a best practice implementation I would recommend.

Replying to exarkun:

Can you explain the purpose of this change more? It's difficult to understand what the desired outcome is (as another non-stdlib-logging module user). My initial reaction is that you should be customizing log events, not having multiple log publishers.

I can sure try. While twisted uses one LogPublisher to accept handle log events and dispatch those to any number of observers, creating a "flat" hierarchy where each observer is equal to the next and each log event gets past to every observer. The stdlib logging module handles this quite different. Although there is a standard logger, the equivalent of a LogPublisher, it supports by design a tree-like structure of loggers with any number of Handlers (equivalent of observers) assigned to any number of loggers in any n-depth tree. In practice, that enables you (as an example) to have a root Logger, of that root a first level node logger per module and a sublogger per class/object/whatever in every module with log events being able to traverse up the tree from lower levels. Because of this from the point of view of a stdlib logging user it is quite natural and often desirable to have a multitude of loggers.

I agree that it is possible to write an observer that can use the default twisted log observer which mimics this behaviour by passing the position of the desired logger in the tree along with the log event into the eventDict, however this would require duplicating code from the twisted PythonLoggingObserver, which I would like to prevent, and I am not yet convinced that it is less work that just moving some lines of code around without any API change.

Also, and this is just my personal opinion, I can see no upside in having a function that only uses the default LogPublisher when it could easily work with any LogPublisher (not use case related) without any obvious downside (to me).

[ralphm]

I have a related use case: writing tests for observers like this:

    def test_emitErr(self):
        observer = MyObserver()
        publisher = log.LogPublisher()
        publisher.addObserver(self.observer.emit)

        try:
            raise TypeError("No foos here")
        except TypeError, e:
            publisher.err(e, "Because")

        # ...

Looks somewhat nicer and closer to the actual usage pattern than:

    def test_emitErr(self):
        observer = MyObserver()
        
        try:
            raise TypeError("No foos here")
        except TypeError:
            f = failure.Failure()
            observer.emit(failure=f,
                          why='Because',
                          isError=1,
                          system='-',
                          time=time.time())

        # ...

Note that I wanted to work with a traceback, so that's why I put this in a try/except block. Arguably, the latter method is better as it doesn't also test the err function itself.

Just using twisted.python.log.err is harder, because the global log publisher is also used by the test runner, which requires flushLoggedErrors to work around.

[ralphm]

On the patch itself:

• The new test doesn't do what you think it does:
  • It only tests the first call. In general, even though existing tests do this: don't use loops in tests. I you really must reduce code duplication in your test, write a helper function.
  • assertIn isn't appropriate for the other two cases.
• The new test has no docstring.
• The new test needs an additional trailing blank line (3 blank lines between classes).
• I'm not sure why this needs a new test, instead of moving some tests from LogTest to LogPublisherTestCase. Note there are more tests in LogTest this could be argued for.
• logerr does not have a test (here), even though this name is redundant and maybe should be deprecated. Touching → testing.
• The existing code for err facilitates startKeepingErrors and ignoreErrors and related code, which itself is untested and deprecated since Twisted 2.5. This seems a like a fine opportunity to remove all that code.
• Please supply a news file.