From 090b98b8307fd924882e78b69df9227b4621ec6b Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Tue, 27 Apr 2021 14:43:17 +0000 Subject: Moving away from custom log levels, to something that's well defined. (#360) * Moving away from custom log levels, to something that's well defined. * Added backward compability to log() as well. * Added an option to force log messages out on screen even if the level is below the log level threashold. * Added force log messages when wrong notation is used. * Added some more length to the deprecated message * Swapped all log levels to use logging. instead. Co-authored-by: Anton Hvornum --- archinstall/lib/output.py | 67 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 17 deletions(-) (limited to 'archinstall/lib/output.py') diff --git a/archinstall/lib/output.py b/archinstall/lib/output.py index 6b184b4b..73819422 100644 --- a/archinstall/lib/output.py +++ b/archinstall/lib/output.py @@ -17,8 +17,32 @@ class LOG_LEVELS: class journald(dict): @abc.abstractmethod - def log(message, level=LOG_LEVELS.Debug): - import systemd.journal + def log(message, level=logging.DEBUG): + try: + import systemd.journal + except ModuleNotFoundError: + return False + + # For backwards compability, convert old style log-levels + # to logging levels (and warn about deprecated usage) + # There's some code re-usage here but that should be fine. + # TODO: Remove these in a few versions: + if level == LOG_LEVELS.Critical: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + level = logging.CRITICAL + elif level == LOG_LEVELS.Error: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + level = logging.ERROR + elif level == LOG_LEVELS.Warning: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + level = logging.WARNING + elif level == LOG_LEVELS.Info: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + level = logging.INFO + elif level == LOG_LEVELS.Debug: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + level = logging.DEBUG + log_adapter = logging.getLogger('archinstall') log_fmt = logging.Formatter("[%(levelname)s]: %(message)s") log_ch = systemd.journal.JournalHandler() @@ -26,19 +50,7 @@ class journald(dict): log_adapter.addHandler(log_ch) log_adapter.setLevel(logging.DEBUG) - if level == LOG_LEVELS.Critical: - log_adapter.critical(message) - elif level == LOG_LEVELS.Error: - log_adapter.error(message) - elif level == LOG_LEVELS.Warning: - log_adapter.warning(message) - elif level == LOG_LEVELS.Info: - log_adapter.info(message) - elif level == LOG_LEVELS.Debug: - log_adapter.debug(message) - else: - # Fallback logger - log_adapter.debug(message) + log_adapter.log(level, message) # TODO: Replace log() for session based logging. class SessionLogging(): @@ -112,17 +124,38 @@ def log(*args, **kwargs): with open(absolute_logfile, 'a') as log_file: log_file.write(f"{orig_string}\n") + # If we assigned a level, try to log it to systemd's journald. # Unless the level is higher than we've decided to output interactively. # (Remember, log files still get *ALL* the output despite level restrictions) if 'level' in kwargs: - if kwargs['level'] > storage.get('LOG_LEVEL', LOG_LEVELS.Info): + # For backwards compability, convert old style log-levels + # to logging levels (and warn about deprecated usage) + # There's some code re-usage here but that should be fine. + # TODO: Remove these in a few versions: + if kwargs['level'] == LOG_LEVELS.Critical: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + kwargs['level'] = logging.CRITICAL + elif kwargs['level'] == LOG_LEVELS.Error: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + kwargs['level'] = logging.ERROR + elif kwargs['level'] == LOG_LEVELS.Warning: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + kwargs['level'] = logging.WARNING + elif kwargs['level'] == LOG_LEVELS.Info: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + kwargs['level'] = logging.INFO + elif kwargs['level'] == LOG_LEVELS.Debug: + log("Deprecated level detected in log message, please use new logging. instead for the following log message:", fg="red", level=logging.ERROR, force=True) + kwargs['level'] = logging.DEBUG + + if kwargs['level'] > storage.get('LOG_LEVEL', logging.INFO) and not 'force' in kwargs: # Level on log message was Debug, but output level is set to Info. # In that case, we'll drop it. return None try: - journald.log(string, level=kwargs.get('level', LOG_LEVELS.Info)) + journald.log(string, level=kwargs.get('level', logging.INFO)) except ModuleNotFoundError: pass # Ignore writing to journald -- cgit v1.2.3-70-g09d2 From a13db6bffd93d317c4acb27ef60dfe25a2301979 Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Wed, 28 Apr 2021 14:20:12 +0200 Subject: Fixing permission error on non-root-runners accessing log() --- archinstall/lib/output.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'archinstall/lib/output.py') diff --git a/archinstall/lib/output.py b/archinstall/lib/output.py index 73819422..6a294f40 100644 --- a/archinstall/lib/output.py +++ b/archinstall/lib/output.py @@ -110,6 +110,8 @@ def log(*args, **kwargs): if not os.path.isfile(absolute_logfile): try: Path(absolute_logfile).parents[0].mkdir(exist_ok=True, parents=True) + with open(absolute_logfile, 'a') as log_file: + log_file.write("") except PermissionError: # Fallback to creating the log file in the current folder err_string = f"Not enough permission to place log file at {absolute_logfile}, creating it in {Path('./').absolute()/filename} instead." -- cgit v1.2.3-70-g09d2 From 04804e6edc13fd4cfcba898767919ae9f187842b Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Wed, 28 Apr 2021 14:28:21 +0200 Subject: Corrected error handling for log creation. --- .gitignore | 1 + archinstall/lib/output.py | 31 ++++++++++++++----------------- 2 files changed, 15 insertions(+), 17 deletions(-) (limited to 'archinstall/lib/output.py') diff --git a/.gitignore b/.gitignore index a21815d6..00f42d12 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ SAFETY_LOCK **/test.py **/archiso /guided.py +/install.log diff --git a/archinstall/lib/output.py b/archinstall/lib/output.py index 6a294f40..872de3d0 100644 --- a/archinstall/lib/output.py +++ b/archinstall/lib/output.py @@ -103,30 +103,27 @@ def log(*args, **kwargs): kwargs = {'fg': 'white', **kwargs} string = stylize_output(string, **kwargs) - # If a logfile is defined in storage, + # If a logfile is defined in storage, # we use that one to output everything if (filename := storage.get('LOG_FILE', None)): absolute_logfile = os.path.join(storage.get('LOG_PATH', './'), filename) - if not os.path.isfile(absolute_logfile): - try: - Path(absolute_logfile).parents[0].mkdir(exist_ok=True, parents=True) - with open(absolute_logfile, 'a') as log_file: - log_file.write("") - except PermissionError: - # Fallback to creating the log file in the current folder - err_string = f"Not enough permission to place log file at {absolute_logfile}, creating it in {Path('./').absolute()/filename} instead." - absolute_logfile = Path('./').absolute()/filename - absolute_logfile.parents[0].mkdir(exist_ok=True) - absolute_logfile = str(absolute_logfile) - storage['LOG_PATH'] = './' - log(err_string, fg="red") - - Path(absolute_logfile).touch() # Overkill? + + try: + Path(absolute_logfile).parents[0].mkdir(exist_ok=True, parents=True) + with open(absolute_logfile, 'a') as log_file: + log_file.write("") + except PermissionError: + # Fallback to creating the log file in the current folder + err_string = f"Not enough permission to place log file at {absolute_logfile}, creating it in {Path('./').absolute()/filename} instead." + absolute_logfile = Path('./').absolute()/filename + absolute_logfile.parents[0].mkdir(exist_ok=True) + absolute_logfile = str(absolute_logfile) + storage['LOG_PATH'] = './' + log(err_string, fg="red") with open(absolute_logfile, 'a') as log_file: log_file.write(f"{orig_string}\n") - # If we assigned a level, try to log it to systemd's journald. # Unless the level is higher than we've decided to output interactively. # (Remember, log files still get *ALL* the output despite level restrictions) -- cgit v1.2.3-70-g09d2 From 754e4b8b61b9c81f1d9f49f96cffd369d78bb777 Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Wed, 28 Apr 2021 14:30:12 +0200 Subject: Corrected one indentation. --- archinstall/lib/output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'archinstall/lib/output.py') diff --git a/archinstall/lib/output.py b/archinstall/lib/output.py index 872de3d0..06d99778 100644 --- a/archinstall/lib/output.py +++ b/archinstall/lib/output.py @@ -103,7 +103,7 @@ def log(*args, **kwargs): kwargs = {'fg': 'white', **kwargs} string = stylize_output(string, **kwargs) - # If a logfile is defined in storage, + # If a logfile is defined in storage, # we use that one to output everything if (filename := storage.get('LOG_FILE', None)): absolute_logfile = os.path.join(storage.get('LOG_PATH', './'), filename) -- cgit v1.2.3-70-g09d2