aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zameer Manji" <zma...@twopensource.com>
Subject Re: Review Request 28742: Simplify logging in the client.
Date Sat, 06 Dec 2014 00:16:35 GMT


> On Dec. 5, 2014, 12: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.

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.


- Zameer


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28742/#review64075
-----------------------------------------------------------


On Dec. 4, 2014, 11:43 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28742/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2014, 11:43 p.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message