Uploaded image for project: 'RHEL Conversions'
  1. RHEL Conversions
  2. RHELC-1171

Migrate away from exceptions.CriticalError

XMLWordPrintable

    • Icon: Epic Epic
    • Resolution: Unresolved
    • Icon: Undefined Undefined
    • None
    • None
    • convert2rhel
    • Refactor: Migrate away from exceptions.CriticalError
    • False
    • False
    • Hide

      None

      Show
      None
    • Testable

      `exceptions.CriticalError` was a stopgap measure to move us away from using `sys.exit ()` inside of Action plugins but we shouldn't rely on it long term. The drawbacks are:

      • Because of its nature as a replacement for sys.exit(), it is communicating directly with a higher level (in this case, the Action Plugin's run() }} method). It is skipping any intermediate functions in the call stack and telling {{Action.run() to abort now. This goes against letting the callers decide how to deal with exceptions.
      • It is a leaky abstraction. CriticalError has fields for id, title, description, diagnosis, and remediation. These are the fields that the Action framework needs to generate good report entries about Action failures, warnings, and information. When we have code that is shared both inside and outside of the Action framework, the code outside of hte Action framework has to have knowledge of these fields in order to choose what it should do.
      • It places the burden of generating these Action specific fields onto the called function but it shouldn't have specific knowledge of who is calling it, just about the data it is processing. A function may also be called from multiple places which may result in awkward choices..For instance. what happens if a function is determining what to place in the ID field but it is called from multiple places within Action plugins. The Action pludins might prefer to use different ids for those but that doesn't map onto the CriticalError data structure.

      What should replace CriticalError? The right way to do this is for the called function to raise an Exception (possibly a prebuilt exception but more likely a custom one) which will bubble up the stack. The exception should contain data that makes sense from the called functions point of view. If nothing handles it in between, then the Action.run() method should catch it and determine what fields (id, description, diagnosis, etc) to use based upon the exception type and perhaps the data contained within.

              Unassigned Unassigned
              tkuratom@redhat.com Toshio Kuratomi
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated: