Convert t.c.log usages to logging.

Review Request #1815 — Created Feb. 22, 2015 and submitted

jsirois
pants
jsirois/commons_log/kill
1153
b559fb0...
pants-reviews
ity, mateor, patricklaw, zundel
Kill remaining BUILD references to t.c.log and configure logging by hand
to be ~exactly how t.c.log had it configured to keep the status quo.

 3rdparty/python/twitter/commons/requirements.txt            |  1 -
 src/python/pants/BUILD.transitional                         |  1 -
 src/python/pants/backend/codegen/tasks/BUILD                |  2 --
 src/python/pants/backend/codegen/tasks/apache_thrift_gen.py |  3 +-
 src/python/pants/backend/codegen/tasks/protobuf_gen.py      |  5 ++-
 src/python/pants/backend/jvm/tasks/BUILD                    |  1 -
 src/python/pants/backend/jvm/tasks/jar_publish.py           |  8 +++--
 src/python/pants/backend/jvm/tasks/nailgun_task.py          |  4 +--
 src/python/pants/base/BUILD                                 |  1 -
 src/python/pants/base/build_environment.py                  | 10 +++---
 src/python/pants/bin/BUILD                                  |  1 -
 src/python/pants/bin/goal_runner.py                         | 84 +++++++++++++++++++++++++++++++-------------------
 src/python/pants/bin/pants_exe.py                           |  1 +
 src/python/pants/goal/context.py                            |  4 +--
 src/python/pants/ivy/BUILD                                  |  1 -
 src/python/pants/ivy/bootstrapper.py                        | 10 +++---
 src/python/pants/java/BUILD                                 |  3 --
 src/python/pants/java/distribution/distribution.py          |  9 ++++--
 src/python/pants/java/executor.py                           | 11 ++++---
 src/python/pants/java/nailgun_executor.py                   | 41 ++++++++++++------------
 20 files changed, 111 insertions(+), 90 deletions(-)
Manual comparison of `-l{debug,info}`, `-d/tmp/pants` and `--quiet` flag
combinations between old and new.

CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/51903573
JS
ZU
  1. Looks good to me! Its a big improvement as-is. I had an idea on how we might configure logging even earlier.

  2. nit: convert % to format while you're at it?

    1. This file and distribution.py have ~10s of % formats - I'd like to hold off and instead fix these all at once in a follow up RB that kills all % use globally so we can be done with this.
  3. src/python/pants/bin/goal_runner.py (Diff revision 2)
     
     

    This call relies on 3 global options:

    level, quiet, and logdir

    I suggest:

    1) Make those options parameters to this method.
    2) Move these 3 options out of register_global_options() into options_bootstraper.py

    Now you can move the self._setup_logging() invocation to line 56, pass the options settings from the bootstrap options and have logging enabled during plugin registration.

    1. This sgtm, but I'd like to do it in a separate RB - this one was just about killing t.c.log and not fixing this pre-existing hole.
  4. nit: change % to format() while you are in here?

  5. 
      
MA
  1. Everything look ok to me. Thanks, John.

  2. 
      
ST
  1. 
      
  2. it's called log on the Context

    1. It is - however prior to this change of the 31 logger acquisitions saved in a variable, 30 are called logger and 1 is a self._log instance member.  I'm going to stick with the prevailing logger name in this change.
  3. 
      
JS
JS
JS
  1. Thanks folks - submitted @ https://github.com/pantsbuild/pants/commit/a7cd9dbf0810051534e200fc3f00175564e1d77c
  2. 
      
JS
Review request changed

Status: Closed (submitted)

IT
  1. Ship It!
  2. 
      
PA
  1. Ship It!
  2. 
      
Loading...