Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify logger.conf and the fallback code #616

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion leapp/libraries/stdlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb
"""
Run a command and return its result as a dict.

The execution of the program and it's results are captured by the audit.
The execution of the command and its result is captured by the audit. The CalledProcessError is raised when
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a special docstring syntax for the definition of what being raised:

@@ -151,8 +151,7 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb
     """
     Run a command and return its result as a dict.
 
-    The execution of the command and its result is captured by the audit. The CalledProcessError is raised when
-    the command exits with non-zero code.
+    The execution of the command and its result is captured by the audit.
 
     :param args: Command to execute
     :type args: list or tuple
@@ -168,6 +167,8 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb
     :type stdin: int, str
     :return: {'stdout' : stdout, 'stderr': stderr, 'signal': signal, 'exit_code': exit_code, 'pid': pid}
     :rtype: dict
+    :raises:
+        CalledProcessError: if the command exits with non-zero status
     """
     if not args:
         message = 'Command to call is missing.'

the command exits with non-zero code.

:param args: Command to execute
:type args: list or tuple
Expand Down
71 changes: 49 additions & 22 deletions leapp/logger/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from leapp.utils.actorapi import get_actor_api, RequestException
from leapp.utils.audit import Audit

_logger = None
_leapp_logger = None


class LeappAuditHandler(logging.Handler):
Expand Down Expand Up @@ -52,39 +52,66 @@ def _remote_emit(self, log_data):


def configure_logger(log_file=None):
global _logger
if not _logger:
"""
Configure loggers as per the description below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


logger: root
level: DEBUG
handler: StreamHandler
level: based on the debug/verbosity options
handler: LeappAuditHandler
level: DEBUG
logger: urllib3
level: WARN
logger: leapp
level: NOTSET
handler: FileHandler
level: DEBUG

:return: The 'leapp' logger
"""
global _leapp_logger
if not _leapp_logger:
_leapp_logger = logging.getLogger('leapp')

log_format = '%(asctime)s.%(msecs)-3d %(levelname)-8s PID: %(process)d %(name)s: %(message)s'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think fo ruser would be convininent to see that this time is UTC time

Suggested change
log_format = '%(asctime)s.%(msecs)-3d %(levelname)-8s PID: %(process)d %(name)s: %(message)s'
log_format = '%(asctime)s.%(msecs)-3d UTC %(levelname)-8s PID: %(process)d %(name)s: %(message)s'

log_date_format = '%Y-%m-%d %H:%M:%S'
path = os.getenv('LEAPP_LOGGER_CONFIG', '/etc/leapp/logger.conf')
logging.Formatter.converter = time.gmtime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this. This is a global state of all loggers. What about the one from the cookbook? (https://docs.python.org/3/howto/logging-cookbook.html#formatting-times-using-utc-gmt-via-configuration)
this could be solved something similar to:

diff --git a/etc/leapp/logger.conf b/etc/leapp/logger.conf
index ee1cf91..66102c4 100644
--- a/etc/leapp/logger.conf
+++ b/etc/leapp/logger.conf
@@ -10,7 +10,7 @@ keys=leapp_audit,stream
 [formatter_leapp]
 format=%(asctime)s.%(msecs)-3d %(levelname)-8s PID: %(process)d %(name)s: %(message)s
 datefmt=%Y-%m-%d %H:%M:%S
-class=logging.Formatter
+class=leapp.logger.UTCFormatter
 
 [logger_urllib3]
 level=WARN


root_logger = logging.getLogger()
root_logger.setLevel(logging.DEBUG)

path = os.getenv('LEAPP_LOGGER_CONFIG', '/etc/leapp/logger.conf')
if path and os.path.isfile(path):
logging.config.fileConfig(path)
else: # Fall back logging configuration
logging.Formatter.converter = time.gmtime
logging.basicConfig(
level=logging.ERROR,
format=log_format,
datefmt=log_date_format,
stream=sys.stderr,
)
stderr_handler = logging.StreamHandler(sys.stderr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to put sys.stderr, it is by default

stderr_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format))
stderr_handler.setLevel(logging.ERROR)
root_logger.addHandler(stderr_handler)

logging.getLogger('urllib3').setLevel(logging.WARN)
handler = LeappAuditHandler()
handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format))
logging.getLogger('leapp').addHandler(handler)

audit_handler = LeappAuditHandler()
audit_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format))
audit_handler.setLevel(logging.DEBUG)
root_logger.addHandler(audit_handler)
bocekm marked this conversation as resolved.
Show resolved Hide resolved

if log_file:
file_handler = logging.FileHandler(os.path.join(get_config().get('logs', 'dir'), log_file))
file_handler.setFormatter(logging.Formatter(fmt=log_format, datefmt=log_date_format))
file_handler.setLevel(logging.DEBUG)
logging.getLogger('leapp').addHandler(file_handler)
_leapp_logger.addHandler(file_handler)

if is_verbose():
for handler in logging.getLogger().handlers:
if isinstance(handler, logging.StreamHandler):
handler.setLevel(logging.DEBUG if is_debug() else logging.INFO)
for handler in root_logger.handlers:
if isinstance(handler, logging.StreamHandler):
if is_debug():
handler.setLevel(logging.DEBUG)
elif is_verbose():
handler.setLevel(logging.INFO)
else:
handler.setLevel(logging.ERROR)

_logger = logging.getLogger('leapp')
_logger.info('Logging has been initialized')
_leapp_logger.info('Logging has been initialized')

return _logger
return _leapp_logger
2 changes: 1 addition & 1 deletion leapp/snactor/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def last_snactor_context(connection=None):
Retrieves the last snactor-run context from the database. It generates a new one if none has been found.
:param connection: Database connection to use instead of the default connection.
:returns: String representing the latest snactor-run context uuid.
:return: String representing the latest snactor-run context uuid.
"""
with get_connection(db=connection) as db:
cursor = db.execute('''
Expand Down