search

abertschi / postcards

A CLI for the Swiss Postcard Creator :postbox:
https://abertschi.ch/blog/2022/receiving-postcards/
MIT License
36 stars 8 forks source link

Introduce proper logging #7

Closed abertschi closed 7 years ago

abertschi commented 7 years ago

As described in #2 branch: https://github.com/abertschi/postcards/compare/master...feature/logging

Liblor commented 7 years ago

Maybe you could introduce a --verbose [LEVEL] flag instead of --trace and --debug, where one can specify the level of verbosity/amount of debug information, e.g.

Level 0 : only errors and success messages (standard) Level 1 : level 0 + debug Level 2 : level 1 + trace

Edit: damn, didn't want to close the issue :sob:

abertschi commented 7 years ago

Maybe you could introduce a --verbose [LEVEL] flag instead of --trace and --debug, where one can specify the level of verbosity/amount of debug information, e.g.

Indeed, that's what I was planning to do :)

Liblor commented 7 years ago

Regarding your commit, it seems a bit odd that one has to configure the logging module before other imports. It's fine as it works, but I wouldn't say that this is best practice ;) (the "issue" is the postcard_creator module, see Configuring Logging for a Library). Oh and the get_logger method (Line 268) is dead code.

but otherwise :+1:

abertschi commented 7 years ago

Thanks for looking at this. 👍

Regarding your commit, it seems a bit odd that one has to configure the logging module before other imports. It's fine as it works, but I wouldn't say that this is best practice ;) (the "issue" is the postcard_creator module, see Configuring Logging for a Library).

You are right, I fixed these "issues" yesterday in the api wrapper I probably will change the import order once the api wrapper is working and the new changes uploaded to pip.

Oh and the get_logger method (Line 268) is dead code.

get_logger is meant for plugins. So, no. I probably should comment this tho :)

Liblor commented 7 years ago

But as the plugins inherit from Postcards, they can access self.logger. There is no need for a getter function?

abertschi commented 7 years ago

I am aware that the getter method is optional. The only reason I wrote it was to be consistent in having only methods in the plugin API. I feel rather indifferent towards keeping it or not though.