cleo icon indicating copy to clipboard operation
cleo copied to clipboard

feat: Added integration with the built-in logging package

Open dylan-robins opened this issue 1 year ago • 6 comments

Closes: #49

This PR integrates the built-in logging package, as listed in the Cleo 3.0 writeup (#415 ). Implementation is of course subject to change, but I thought I'd get the ball rolling :)

Here's what I did:

  • Added an OutputHandler class, allowing logging to emit LogRecords to a cleo.io.outputs.output.Output
  • Added a method to cleo.application.Application which attaches an OutputHandler to the root logger, and configures it to have a log level that is coherent with Cleo's one.
  • Added unit tests ensuring all this works as expected.

Here are some open points that I know we need to discuss:

  • How should we map logging's log levels to Cleo's verbosity levels? Current implementation is as follows:
    • Quiet: CRITICAL (but nothing gets printed anyway so whatever)
    • Normal: WARNING
    • Verbose: INFO
    • Very verbose: DEBUG
    • Debug: DEBUG
  • Formatting:
    • Should the stuff coming from the logging handler have some sort of prefix or something to distinguish them from what is coming directly from output.write_line()?
    • The formatter styles for error, warning, info and debug need to be defined.

dylan-robins avatar Nov 03 '24 23:11 dylan-robins

Hey, first of all, thank you for your contribution. A couple of things to start with:

  1. Logging integration shouldn't be automatic. We should only provide a logging.Handler class (CleoHandler maybe?) and leave the rest to the user.
  2. I believe we can have a deeper integration and provide more from Cleo than just wrapping io.write into a logging handler. See what Rich does in their logging module. It won't be 1:1, but it should give you some ideas.
  3. The code should be moved to cleo.logging module.

Secrus avatar Nov 18 '24 14:11 Secrus

Thanks for the feedback, I'll look into everything you said and update my branch accordingly. I'll let you know when I have something worth reviewing!

dylan-robins avatar Nov 18 '24 16:11 dylan-robins

  • [ ] Logging integration shouldn't be automatic. We should only provide a logging.Handler class (CleoHandler maybe?) and leave the rest to the user.
  • [ ] I believe we can have a deeper integration and provide more from Cleo than just wrapping io.write into a logging handler. See what Rich does in their logging module. It won't be 1:1, but it should give you some ideas.
  • [x] The code should be moved to cleo.logging module.

The non-automatic integration is giving me a bit of a headache. The CleoHandler class needs a reference to the application's Output object in order to be able to write to it, however Application.io doesn't exist until the application is running. This means that the user can't initialise the handler before calling app.run()...

Any thoughts on how to proceed?

I have two approaches in mind:

  1. Refactor cleo.Application to make it possible to inject a pre-initialized io object
  2. Keep the _initialize_logger() method I added to Application, but make it opt-in using an argument to __init__

dylan-robins avatar Jan 04 '25 10:01 dylan-robins

Erm forget that last comment, I'm dumb, obviously I can just create my Output object and pass it to the Application.run function. Duh 🙃

dylan-robins avatar Jan 04 '25 10:01 dylan-robins

  • [x] Logging integration shouldn't be automatic. We should only provide a logging.Handler class (CleoHandler maybe?) and leave the rest to the user.
  • [ ] I believe we can have a deeper integration and provide more from Cleo than just wrapping io.write into a logging handler. See what Rich does in their logging module. It won't be 1:1, but it should give you some ideas.
  • [x] The code should be moved to cleo.logging module.

Integration is now fully handled on the user side. I've updated the tests to reflect this, and this is a working example of user code:

import logging
import sys

from cleo.application import Application
from cleo.io.outputs.stream_output import StreamOutput
from cleo.logging.cleo_handler import CleoHandler
from tests.fixtures.foo4_command import Foo4Command


if __name__ == "__main__":

    # Configure a cleo single-command application
    app = Application()
    cmd = Foo4Command()
    app.add(cmd)
    app._default_command = cmd.name

    # Create the custom handler that'll redirect logging's prints to be run through Cleo's output
    output = StreamOutput(sys.stdout)
    handler = CleoHandler(output)
    # The user can set their format as usual, albeit with support for cleo format strings!
    fmt = logging.Formatter(
        "<fg=light_gray;options=reverse>%(asctime)s</> - %(levelname)s: %(message)s",
        datefmt='%m/%d/%Y %I:%M:%S %p',
    )
    handler.setFormatter(fmt)

    # Register the handler
    root = logging.getLogger()
    root.addHandler(handler)
    root.setLevel(logging.NOTSET) # IMPORTANT so that the root logger respects cleo's verbosity levels

    # run the cleo application
    app.run(output=output, error_output=output)

Result: image

image

I'll start looking into what Rich do for formatting and continue updating this PR

dylan-robins avatar Jan 04 '25 12:01 dylan-robins

Thanks for working on that @dylan-robins, it looks great so far.

Secrus avatar Jan 12 '25 17:01 Secrus

Hi Secrus, coming back to this after a long time away. Still loving Cleo 😄

  • [x] Logging integration shouldn't be automatic. We should only provide a logging.Handler class (CleoHandler maybe?) and leave the rest to the user.
  • [x] I believe we can have a deeper integration and provide more from Cleo than just wrapping io.write into a logging handler. See what Rich does in their logging module. It won't be 1:1, but it should give you some ideas.
  • [x] The code should be moved to cleo.logging module.

I've added some nicer features inspired by Rich. As mentioned in #471, I've added the warning and debug styles from Poetry, and added the code required to handle exceptions using the existing ExceptionTrace renderer in Cleo. I've also simplified my example, aligning it very close to what's in Rich's documentation.

How does this look? Anything else you'd like to add?

# tmp.py
from __future__ import annotations

import logging
import sys

from cleo.application import Application
from cleo.io.outputs.stream_output import StreamOutput
from cleo.logging.cleo_handler import CleoHandler
from tests.fixtures.foo4_command import Foo4Command


if __name__ == "__main__":

    # Configure a cleo single-command application
    app = Application()
    cmd = Foo4Command()
    app.add(cmd)
    app._default_command = cmd.name

    # Create the custom handler that'll redirect logging's prints to be run through Cleo's output
    output = StreamOutput(sys.stdout)
    logging.basicConfig(
        level="NOTSET",
        format="<fg=light_gray;options=reverse>%(asctime)s</> - %(levelname)s: %(message)s",
        datefmt='%m/%d/%Y %I:%M:%S %p',
        handlers=[CleoHandler(output)]
    )

    # run the cleo application
    app.run(output=output, error_output=output)

image image image image

dylan-robins avatar Aug 24 '25 16:08 dylan-robins

@dylan-robins the code looks fine for me, at least for starters. Would you be willing to write some basic docs (single file, describing the how-to of the integration etc)?

Secrus avatar Oct 01 '25 22:10 Secrus