neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
12 stars 14 forks source link

Add basic logging mechanism #9

Closed netoarmando closed 4 years ago

netoarmando commented 4 years ago

Configure a logger to be used in the whole application.


Based on the table [here](https://docs.python.org/3/howto/logging.html#when-to-use-logging, I wonder if we should keep all the prints for stdout and stderr messages instead of replacing them.

pvoborni commented 4 years ago

It will probably depend on the action to run. Actions like 'up' and 'destroy' should probably log almost everything. They will be used in CIs so the log should be available. I think that we can log more info (e.g. info from providers) to DEBUG level than we now print to console.

For human 'console' actions like 'list' and 'ssh' we probably will mostly log to INFO and print.

netoarmando commented 4 years ago

It will probably depend on the action to run. Actions like 'up' and 'destroy' should probably log almost everything. They will be used in CIs so the log should be available. I think that we can log more info (e.g. info from providers) to DEBUG level than we now print to console.

For human 'console' actions like 'list' and 'ssh' we probably will mostly log to INFO and print.

Agreed! Let me enhance this.

pvoborni commented 4 years ago

I see that there is used pattern:

        logger.info(f"Hosts to delete: {names}")
        print(f"Hosts to delete: {names}")

Do we want this code duplicity with strings?

In the previous comment I thought that we will:

I'm also open to not have the console logger but in such case we might want to have some utility function like

def print_log(logger, msg, level="INFO"):    
    file = sys.stdout
    if level in ("ERROR", "CRITICAL"):
       file = syst.stderr
    print(msg, file=file)
    logger.log(level, msg)
netoarmando commented 4 years ago

@pvoborni it was hard to feel comfortable with the idea of logging+printing a CLI application. Specially because the common practice is to log to stdout. But I think your points now made it clear to me.

About the duplication, I wonder if most of the strings would be the same or not. However, I guess all of this is fixed by having a custom print function. It looks strange, but I think is the best solution.

netoarmando commented 4 years ago

Earlier today @pvoborni and I talked about this and we agreed to the following points:

Tiboris commented 4 years ago

@pvoborni Let us merge this as well. Thanks @netoarmando