-
Epic
-
Resolution: Unresolved
-
Undefined
-
None
-
None
-
None
-
Refactor: Migrate away from logger.critical exiting
-
False
-
False
-
-
50% To Do, 0% In Progress, 50% Done
-
Testable
Problem
The flow of execution in Python should be that one function (the caller) invokes another function (the callee) and when the callee exits, execution returns to the caller at the line where it invoked the callee. In the normal case where there was no error, the caller will then proceed to the next line of code. In the exception case, the caller will enter an exception handler (the block of code inside an except:) to handle the error or allow the exception to bubble up to its caller which can make the same decision as to whether it knows how to deal with the error or allow the exception to bubble up further.
Our logger.critical() violates this flow of execution by calling sys.exit() which means that anytime a function calls it, it stops execution of convert2rhel. This has been convenient in the past but it breaks the usual way that exceptions are supposed to work. That makes it harder to understand the flow of execution in convert2rhel and causes a mismatch when we try to fit existing code into new frameworks (for instance, when this happens in the Action framework).
This Epic exists to plan, list, and track how to refactor our code to move away from this.
Action plan
- Add a new custom log method for critical_no_exit().
- Port Action framework critical() calls to use the critical_no_exit() instead. (These are nice to have for https://issues.redhat.com/browse/RHELC-965 )
- Add a new logger, {{logger.critical_with_exit }}that does what the existing{{ logger.critical() }}does now.
- Add a new LogLevel higher than 50: https://github.com/oamg/convert2rhel/blob/aeca505239d20a04e8e68d76ab8cb9751e7c7cdf/convert2rhel/logger.py#L39
- Search logger.py for critical and add the appropriate code for critical_with_exit() there.
- Move all existing usage of critical() in our code to critical_with_exit()
- Rename critical_no_exit() to critical()
- At this point, new code should use logger.critical() and should expect that the log call will not exit.
- At this point, new code should use logger.critical() and should expect that the log call will not exit.
-
- This should perform the find and replace:{{ find . -name '*.py' -exec sed -i s/critical_no_exit/critical/g {} \;}}
- Port individual uses of logger.critical_with_exit() to {{logger.critical(). }}For each usage this require:
- Switching the call
- Defining or deciding on an appropriate exception to raise instead
- Auditing all the code's callers to catch and deal with the new exception if appropriate.
- When all uses of logger.critical() have been ported, remove critical_with_exit() and supporting code in logger.py
- relates to
-
RHELC-1171 Migrate away from exceptions.CriticalError
- New