Hi,

I'm currently investigating a bug with the cache. To be able to track this (and other issues in the future) down, the attached patch introduces a common logger for offpunk (and it's modules). The patch also updates some existing error handling functions to use the logger. The logging level can be adjusted by setting it in offlogger.py. If there is need in the future, a switch could also be added to the command line arguments. 

Thanks,
Von

On 23/11/28 05:53, vonhohenheiden@tutanota.com wrote:
>Hi,
>
>I'm currently investigating a bug with the cache. To be able to track 
>this (and other issues in the future) down, the attached patch 
>introduces a common logger for offpunk (and it's modules). The patch 
>also updates some existing error handling functions to use the logger. 
>The logging level can be adjusted by setting it in offlogger.py. If 
>there is need in the future, a switch could also be added to the 
>command line arguments.

Thanks for the patch. I will test and try it a bit more in depth before 
accepting it.

Any review from others is also welcome.

Don’t hesitate to discuss the bug you are investigating!

On 23/11/28 05:53, vonhohenheiden@tutanota.com wrote:
>Hi,
>
>I'm currently investigating a bug with the cache. To be able to track this (and other issues in the future) down, the attached patch introduces a common logger for offpunk (and it's modules). The patch also updates some existing error handling functions to use the logger. The logging level can be adjusted by setting it in offlogger.py. If there is need in the future, a switch could also be added to the command line arguments.
>
Here are already some feedbacks

1) The patch import the logging module. Could you confirm that this 
module is part of all python installations? Do you see any potential 
case where logging might not be available?

2) The patche completely break the distinction between different kind of 
error in offpunk. There are:
- user errors (bad command, missing dependency, unset variables, etc)
- networking errors

By design, user errors are always displayed, as an immediate feedback. 
Networking error are only displayed when running interactively 

offpunk.py:315
params["print_error"] = not self.sync_only 


Your patch completely break that behaviour. It merges all kind of errors 
and breaks the fact that network errors should be hidden during 
sync_only.


3) Currently, when a fetch failed, the cache stores the error verbatim. 
This is a hack that should be improved but this is the situation now. By 
introducing a logger, you probably break the current behaviour regarding 
cache.

See ~lioploum/offpunk#3  (and ~lioploum/offpunk#30 )


4) While I agree that the "if print_error: print("blabla") could be 
refactorised in one single print_error() function, I fails to see the 
benefit of adding a logger, which add complexity (nothing is easier than 
a simple "print" statement in python).