Finish updating rbt patch to localize and use logging for output.

Review Request #10461 — Created March 21, 2019 and updated

chipx86
RBTools
master
rbtools

This finishes localizing all the strings in rbt patch, preparing for
a future in which we might someday offer translations. It also moves us
to using logging for all remaining output.

Tested the various conditions, making sure the output was correct.

Summary
Finish updating rbt patch to localize and use logging for output.
Description From Last Updated

Hmmm. I'm really not sure about using logger.info for what is fundamentally expected to be stdout output. The more I ...

daviddavid
Checks run (2 succeeded)
flake8 passed.
JSHint passed.
david
  1. 
      
  2. rbtools/commands/patch.py (Diff revision 1)
     
     
     
     

    Hmmm. I'm really not sure about using logger.info for what is fundamentally expected to be stdout output.

    The more I work with the rbtools codebase the more I feel like regular/error output should use print/sys.stderr.write and logging should fundamentally just be about logging.

    1. I've wrestled with this too, and I don't really love either solution, but I think something other than an explicit print/sys.*.write is the right direction going forward, for reasons I'll go into. We should figure this out before too much more work is done in either direction.

      There's a couple major goals I have in mind going forward for RBTools:

      1. Be able to more easily script and consume using RBTools commands (using things like --json, which I still want to get in).
      2. Being able to wrap/embed commands from Python scripts or other tools, such as IDEs.

      In both scenarios, it helps to have control over output. In the --json case, I'd like to make JSON output support a core part of RBTools, so that any command can easily say "here's the data I've collected" and have that presented to the caller without each command having to wrap every print statement with a if not self.options.show_json. Being able to have the core Command class just shut off standard output, leaving only warnings/errors going to stderr by default and then explicitly printing the JSON result, makes that so much easier to provide without there being mistakes.

      Consuming commands in another process benefits from control over output as well. If someone's building a Python tool that performs a set of operations, making use of our patching/landing or status updating or something via the commands (as there's a lot of logic there we don't want duplicated), they very well may not want the output going to stdout. They might want to actually log it to a file, or in the case of an IDE show something in a log pane widget that's more than just a representation of a terminal.

      Using something like logging gives us a lot of flexibility here. All info/warnings/errors can be printed to the console, or shut off, or changed in their representation (JSON stream of logs to an IDE, for instance), or just embedded in part in JSON output. If we used print and sys.stderr.write, then we have minimal control over this, and to give any of the above advantages, we have to redirect these to logging or something anyway.

      logging also gives us the advantage of named loggers. A consumer can turn off or change logging coming from particular subsystems without affecting their own logging or output to the console.

      So I feel given the options and given where RBTools would be useful going forward, logging is the best option right now. Another option would be something like what Django's management commands have, where there's Command.stdout and Command.stderr that default to the sys equivalents but could then be redirected. However, stderr doesn't differentiate between warnings and errors, and I think logging.warning/error are the correct things to use for those.

  3. 
      
Loading...