[PATCH] Added chunkname prefix to some elements of the debug log.

Richard Maw richard.maw at codethink.co.uk
Mon Oct 7 13:57:13 BST 2013


On Fri, Oct 04, 2013 at 03:25:15PM +0100, Daniel Firth wrote:
> diff --git a/morphlib/app.py b/morphlib/app.py
> index a0833d4..6c39d38 100644
> --- a/morphlib/app.py
> +++ b/morphlib/app.py
> @@ -417,7 +417,7 @@ class Morph(cliapp.Application):
>  
>          # Log the environment.
>          prev = getattr(self, 'prev_env', {})
> -        morphlib.util.log_dict_diff(kwargs['env'], prev)
> +        morphlib.util.log_dict_diff(self, kwargs['env'], prev)
>          self.prev_env = kwargs['env']
>  
>          # run the command line
> diff --git a/morphlib/util.py b/morphlib/util.py
> index 19c0046..8a2cfd7 100644
> --- a/morphlib/util.py
> +++ b/morphlib/util.py
> @@ -171,20 +171,20 @@ def new_repo_caches(app):  # pragma: no cover
>      return lrc, rrc
>  
>  
> -def log_dict_diff(cur, pre): # pragma: no cover
> +def log_dict_diff(app, cur, pre): # pragma: no cover
>      '''Log the differences between two dicts to debug log'''
>      dictA = cur
>      dictB = pre
>      for key in dictA.keys():
>          if key not in dictB:
> -            logging.debug("New environment: %s = %s" % (key, dictA[key]))
> +            app.status(msg="New environment: %s = %s" % (key, dictA[key]))
>          elif dictA[key] != dictB[key]:
> -            logging.debug(
> +            app.status(msg= \
>                  "Environment changed: %(key)s = %(valA)s to %(key)s = %(valB)s"
>                  % {"key": key, "valA": dictA[key], "valB": dictB[key]})
>      for key in dictB.keys():
>          if key not in dictA:
> -            logging.debug("Environment removed:  %s = %s" % (key, dictB[key]))
> +            app.status(msg="Environment removed:  %s = %s" % (key, dictB[key]))

Instead of passing the whole app object to the log_dict_diff function,
I would prefer if you only passed a reference to the status method.

I dislike the amount of coupling between the app and all the surrounding
functions just to be able to provide status output, just the status
function would be a better design to me.

I'm voting +0 to this, since it's a sin committed all across the codebase.



More information about the baserock-dev mailing list