[PATCH 2 of 6 v2] helpers: add automatic logging to Flash

[Mads Kiilerich]

On 06/04/2015 09:52 PM, Thomas De Schampheleire wrote:
> What you mean is this, right?
>
>          if log_category is None:
>              log_category = {
>                  'notice': 'info',
>                  'warning': 'info',
>                  'error': 'error',
>                  'success': 'debug',
>              }[category]
>
>          {
>              'debug': log.debug,
>              'info': log.info,
>              'warning': log.warning,
>              'error': log.error,
>          }[log_category]('Flash: %s' % message)
>
> Do you really find this more readable than with the intermediate variables?
> For example in the second case, I find it much less obvious that it's
> a function call.
>
> Regarding placing these variables outside the function: what is the
> reasoning here? Since they are only used and relevant for that
> function, isn't it more logical to scope it as such?

Python "constants" are executed when they are assigned, so it is 
generally better to make them as global as possible and avoid defining 
them inside functions or loops. I also like that reading the program 
doesn't get interrupted by things not directly related to the problem flow.


But ... from what I have seen, I think I really would suggest something 
like this:

     def __call__(self, message, category=None, ignore_duplicate=False, 
logf=None):
         '''Show a message to the user _and_ log it

         category: notice (default), warning, error, success
         logf: a custom log function - such as log.debug

         If logf is not set, the message is logged as log.info, except 
success
         is logged as log.debug.
         '''
         if logf is None:
             logf = log.info
             if category == 'success':
                 logf = log.debug

         logf('Flash %s: %s', category, message)

         super(Flash, self).__call__(message, category, ignore_duplicate)

(because I think flash error messages also only should be shown to 
admins if they configure info logging.)

/Mads