> On Dec. 5, 2014, 8:37 p.m., Bill Farner wrote: > > What would it take to massage the log format? I'd love to drop the > > `INFO:root:` prefix to every line. > > > > Personally, i find it noisy for a command line tool to include the log > > level on each line of output, though others may feel differently. > > > > However, i'm pretty firm on omitting the `root` part. > > Zameer Manji wrote: > Technically we don't need to do a lot of work to add a new log Formatter. > However I think logging should be used for debugging purposes and maintaining > the logger name (root/requests/etc) is very useful. If the logging info is > annoying and we show it we should evaluate the logging level and if it should > be visible during normal operation. > > I added a TODO to only show messages from WARN+. Currently it is INFO > because the client side updater uses logging.info to send information to the > user. I am willing to followup this diff with cleaning up the > logging/printing so we don't use it to display useful information to the user > during normal operation. > > Bill Farner wrote: > The logger name might be useful to us, but i don't think it's useful to > our users. Libraries and refactors are implementation details they should be > insulated from. > > Zameer Manji wrote: > Our users shouldn't see log messages in regular operation so I don't see > the need to remove the logger name from the message and make our lives harder > when debugging.
Per offline discussion, please adapt this change to match the current client output so we can tackle the broader output discussion separately. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28742/#review64075 ----------------------------------------------------------- On Dec. 5, 2014, 7:43 a.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28742/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2014, 7:43 a.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-919 > https://issues.apache.org/jira/browse/AURORA-919 > > > Repository: aurora > > > Description > ------- > > This patch makes multiple changes to simplify the logging done in the Aurora > client: > 1. Remove the TRANSCRIPT log level and replaced all instances with the > standard Python DEBUG level. > 2. Remove the custom "aurora_client" logger. This logger was designed to give > each invocation of the client a unique id and record the username of the user > with the intention that a hook could take this information and ship it to the > cluster administer. However a hook could capture logs by adding a handler to > the root log handler and generate a unique id itself. > 3. Remove the 'print_log' method of the context and replaced all callers with > the standard python logging facilities. > 4. Removed duplicate printing/logging messages by just printing the > information to the user. > 5. Removed the custom PlainFormatter implementation and replaced it with > Python's default formatter. > 6. Replaced the "--verbose-logging" and "--logging-level" flags with a single > "--verbose/-v" flag which enables DEBUG logging. Without this flag the user > sees INFO and up. > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/BUILD > ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 > src/main/python/apache/aurora/client/cli/__init__.py > 6e553d8af459e575b2d62282a3bc0d1e266203d8 > src/main/python/apache/aurora/client/cli/command_hooks.py > aa850bf941bede1d3bd8aae4811cb094ba77965f > src/main/python/apache/aurora/client/cli/context.py > 51c7d24dca664e476e62f1864d095416dfab70e4 > src/main/python/apache/aurora/client/cli/jobs.py > 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf > src/main/python/apache/aurora/client/cli/logsetup.py > 55d99c42f643910db0bf3c24022596383e160276 > src/main/python/apache/aurora/client/cli/standalone_client.py > b7c8de66d6e4664b536911f826e36a984e8d0fef > src/main/python/apache/aurora/client/cli/task.py > 91175facdc8ccccc9fd59ab66781f86ee8b5940a > src/test/python/apache/aurora/client/cli/BUILD > e1f9ebf96774b8f5c75de8570c6ba87d953ab649 > src/test/python/apache/aurora/client/cli/test_logging.py > 6285fbb07442291c2dc4096e68eb285c98994097 > src/test/python/apache/aurora/client/cli/test_plugins.py > 7a0a31818cbc57de952d7817f8e7c8fa1e84b25a > src/test/python/apache/aurora/client/cli/test_task.py > c69a624ec7063973d365846f7df3516047ceeb68 > > Diff: https://reviews.apache.org/r/28742/diff/ > > > Testing > ------- > > ./pants ./src/test/python/apache/aurora:: > > ```` > vagrant@192:~$ aurora2 config list ./aurora/examples/jobs/hello_world.aurora > jobs=[devcluster/www-data/prod/hello] > > vagrant@192:~$ aurora2 job create devcluster/www-data/prod/hello > ./aurora/examples/jobs/hello_world.aurora > INFO:root:Creating job hello > INFO:root:Checking status of devcluster/www-data/prod/hello > job create succeeded: job > url=http://192.168.33.7:8081/scheduler/www-data/prod/hello > vagrant@192:~$ aurora job list devcluster/* > Must supply one of the following commands: cancel_update, create, diff, > get_quota, help, inspect, kill, killall, list_jobs, open, restart, run, ssh, > start_cron, status, update, version > > vagrant@192:~$ aurora2 job list devcluster/* > INFO:root:Retrieving jobs for role None > devcluster/www-data/prod/hello > > vagrant@192:~$ aurora2 job update devcluster/www-data/prod/hello > ./aurora/examples/jobs/hello_world.aurora > INFO:root:Updating job: hello > INFO:root:Instances to update: [0] > INFO:root:Processing in parallel with 1 worker thread(s) > INFO:root:Examining instance: 0 > INFO:root:Skipping unchanged instance: 0 > INFO:root:Update successful > Update completed successfully > > vagrant@192:~$ aurora2 job killall devcluster/www-data/prod/hello > INFO:root:Checking status of devcluster/www-data/prod/hello > INFO:root:Killing tasks for job: devcluster/www-data/prod/hello > INFO:root:Instances to be killed: [0] > Successfully killed shards [0] > job killall succeeded > ```` > > > Thanks, > > Zameer Manji > >
