Logrevision - Githubissues

segaura commented 3 years ago

Closes #37

NOTE: this pull request is about log, so it spreads over most of the python files, I am not able to test everything (e.g. openhub driver, HPSUD mode) so more peer inspection or peer test is needed

This pull request removes the printd() method

    def printd(self, level, msg):        
        if self.logger:
            if level == 'warning':
                self.logger.warning(msg)
            elif level == 'error':
                self.logger.error(msg)
            elif level == 'info':
                self.logger.info(msg)
            elif level == 'exception':
                self.logger.exception(msg)
        else:
            if self.driver != "HPSUD":
                print("%s - %s" % (level, msg))

as it can be seen it has different problems:

level is passed as string
stardard logging levels, e.g. DEBUG, are missing
if using a logger the logging level is respected, if not everything is printed to stdout discouraging developers to use debug, info and warning ...moreover in this only case the format is different
"HPSUD mode" is without log and this is obtained accessing an "external object"

To obtain printd() flexibility, a single logger is instructed to use:

FileHandler when -g option requires to save log to file
StreamHandler for default behavious when log is written to stdout
NullHandler when no log has to be written due to "HPSUD mode"

This "one-only-logger" approach enable all the log to use the same global settings for LOG LEVEL.

Having an effetive LOG LEVEL enables to change it via cli option, the --log_level has been introduced for this.

A single unique formatter is defined in advance.

Two info log line were added as proof of concept, they are one for loading of command definitions JSON file and the other for loading of command trasnlations CSV file.

Default level has been left as-is: ERROR.

In addition to log revision this pull request includes:

little cosmetic fix, like typos
a "help" improvement, now along each "non-writable" command description a "(readonly)" suffix is attached ...and this is because there are long more writable command than there are readonly command

Many things can be further improved, for instance:

starting to log before and during options parsing
introducing a method to write a message to stdout and log at the same time
enabling logformatter to be read from conf file of cli option
wrapping all the residual print("some message") to allow more control and avoid to mix them with real output (e.g. JSON)

Spanni26 commented 3 years ago

Can you create this request new an merge against the testing branch? I think it would be better. Changes can be testet without harming the master branch.

segaura commented 3 years ago

I did. Long better, thank you.

Spanni26 commented 3 years ago

Merged :)

Spanni26 commented 3 years ago

When testing is stable, we can merge it into master and create a new testing.

Am 11.02.2021 um 15:21 schrieb segaura:

I did. Long better, thank you.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Spanni26/pyHPSU/pull/38#issuecomment-777494057, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2I5HHTA234XWDVDFKZEJ3S6PRWPANCNFSM4XO4GDTA.

segaura commented 3 years ago

Cool, how test is managed? I ask both to contribute to actual test, both for a possible revision, introducing unit tests, mock and so on.

Spanni26 commented 3 years ago

I don't think that unit tests make sense. The project is too smal for this, too less contribution.

And now, with vannis new project, it will be a dead project soon.

To test it, I would do a "git clone2 on the testing branch and install it via install.sh

Regards

Daniel

Am 11.02.2021 um 17:03 schrieb segaura:

Cool, how test is managed? I ask both to contribute to actual test, both for a possible revision, introducing unit tests, mock and so on.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Spanni26/pyHPSU/pull/38#issuecomment-777602620, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2I5HHSY43C6MP7SVX3FYTS6P5TJANCNFSM4XO4GDTA.

martinbischofff commented 3 years ago

And now, with vannis new project, it will be a dead project soon.

:-o Can you provide me a link to that new project you mentioned? I'm using your project and I'm quite happy with it. I was glad to see the new contributions.

Spanni26 commented 3 years ago

It is at the very first beginning. There isn't a hompage etc. et the moment.

We'll see.....

Am 11.02.2021 um 18:25 schrieb martinbischofff:

And now, with vannis new project, it will be a dead project soon.
:-o Can you provide me a link to that new project you mentioned? I'm using your project and I'm quite happy with it. I was glad to see the new contributions.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Spanni26/pyHPSU/pull/38#issuecomment-777658808, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2I5HE5JYA6GEGNEV5MY73S6QHIFANCNFSM4XO4GDTA.

Spanni26 / pyHPSU

Logrevision #38