owned this note
owned this note
Published
Linked with GitHub
Some thoughts on the design of the framework to handle these:
## Design criteria
1. We need to have separate returns for final results (SUCCESS, ERROR, and SKIP) because there must be one and only one of those after run() finishes.
2. final result messages should be a list so that adding multiple messages to SUCCESS can be nicely formatted.
3. WARNING, and other things that can occur multiple times should be stored in a new list attribute on the Action class.
4. When evaluating logger.warning and logger.info, decide whether to set it as a SUCCESS messages or WARNING message. success messages will not be shown in convresion, only in analyze. warning messages will be shown in both.
5. add_result() should be renamed since we want results to be final results only. Maybe add_message()?
## New Action class
``` python
STATUS_CODE = {
"SUCCESS": 0,
"WARNING": 51,
"OVERRIDABLE": 101,
"SKIP": 151,
"ERROR": 202,
}
def _action_defaults_to_success(func):
"""
Decorator to set default values for return values from this change.
The way the Action class returns values is modelled on
:class:`subprocess.Popen` in that all the data that is returned are set on
the object's instance after :meth:`run` is called. This decorator
sets the functions to return values to success if the run() method did
not explicitly return something different.
"""
@wraps(func)
def wrapper(self, *args, **kwargs):
return_value = func(self, *args, **kwargs)
if self.result is None:
self.result = ActionResult("SUCCESS")
return return_value
return wrapper
@six.add_metaclass(abc.ABCMeta)
class Action:
"""Base class for writing a check."""
# Once we can limit to Python3-3.3+ we can use this instead:
# @property
# @abc.abstractmethod
# def id(self):
@abc.abstractproperty # pylint: disable=deprecated-decorator
def id(self):
"""
This should be replaced by a simple class attribute.
It is a short string that uniquely identifies the Action.
For instance::
class Convert2rhelLatest(Action):
id = "C2R_LATEST"
`id` will be combined with `error_code` from the exception parameter
list to create a unique key per error that can be used by other tools
to tell what went wrong.
"""
#: Override dependencies with a Sequence that contains other
#: :class:`Action`s :attr:`Action.id`s that must be run before this one.
#: The :attr:`Action.id`s can be specified as string literals; you don't
#: have to import the class to reference them in the Sequence.
dependencies = ()
def __init__(self):
"""
The attributes set here should be set when the run() method returns.
They represent whether the Change succeeded or failed and if it failed,
gives useful information to the user.
After the Action is run, result will contain an ActionMessage telling whether the Action was a SUCCESS, FAILURE, or was SKIPPED.
If there were any warnings or other messages that the user should see in the report, the messages list will contain an ActionMessage for each message.
"""
self._has_run = False
self._result = None
self.messages = []
@_action_defaults_to_success
@abc.abstractmethod
def run(self):
"""
The method that performs the action.
.. note:: This method should set :attr:`status`, :attr:`message`, and
attr:`error_id` before returning. The @action_defaults_to_success
decorator takes care of setting a default success status but you
can either add more information (for instance, a message to
display to the user) or make additional changes to return an error
instead.
"""
if self._has_run:
raise ActionError("Action %s has already run" % self.id)
self._has_run = True
@property
def result(self):
return self._result
@result.setter
def result(self, action_message):
"""
Make sure the correct policy for result messages is used.
"""
if not isinstance(action_message, ActionResult):
raise Exception()
self._result = action_message
def set_result(self, message_id, message_level, message=""):
"""
Helper method that sets the resulting values for status, error_id and message.
:param message_id: The message_id to identify the result.
:type message_id: str
:param message_level: The status_code of the result .
:type: message_level: str
:param message: The message to be set.
:type message: str | None
"""
if message_level not in ("FAILURE", "SKIP", "SUCCESS"):
raise KeyError("The message_level of result must be FAILURE, SKIP, or SUCCESS.")
result = ActionResult(message_id, message_level, message)
self.result = result
def add_message(self, message_id, message_level, message):
"""
Helper method that adds the resulting values for level, id and message of a warning or info log message.
:param message_id: The message_id to identify the message.
:type message_id: str
:param message_level: The message_level to be set.
:type: message_level: str
:param message: The message to be set.
:type message: str | None
"""
msg = ActionMessage(message_id, message_level, message)
self.messages.append(msg)
class ActionMessageBase:
def __init__(self, message_level, message_id=None, message=""):
self.message_id = message_id
self.message_level = STATUS_CODE[message_level]
self.message = message
def to_dict(self):
return {
"message_id": self.message_id,
"message_level": self.message_level,
"message": self.message,
}
class ActionMessage(ActionMessageBase):
def __init__(self, message_level, message_id=None, message=""):
if not (message_id and message_level and message):
raise Exception()
if message_level > STATUS_CODE["SUCCESS"] and message_level < STATUS_CODE["SKIP"]:
raise Exception()
super(ActionMessage, self).__init__(message_level, message_id, message)
class ActionResult(ActionMessageBase):
def __init__(self, message_level, message_id=None, message=""):
if action_message.message_level >= "SKIP":
if not action_message.message_id:
raise Exception("Non-success results require a message_id")
if not action_message.message:
raise Exception("Non-success results require a message")
elif not action_message.message_level <= "SUCCESS":
raise Exception("The message_level for result must be SKIP or more fatal or SUCCESS.")
```
## Modifications due to Action class changes
1. Action modules: find calls to self.set_result() and make sure the keyword arguments use the new parameter names, message_id, message_level, and message.
2. In the warnings PR, change the calls to add_result() to match with the new add_message()
## Modifications to the rest of the Action framework and main
1. actions.run_actions() => returns results as a dict
2. Stage.run() needs to be modified for where it calls set_result() to use the new parameter names
3. Stage.run() needs to be modified where it checks action.status (to use action.result.message_level)
4. Modify actions.find_actions_of_severity
5. Modify actions.report.summary
``` python
def run_actions():
"""
Run all of the pre-ponr Actions.
This function runs the Actions that occur before the Point of no Return.
"""
# Stages are created in the opposite order that they are run in so that
# each Stage can know about the Stage that comes after it (via the
# next_stage parameter).
#
# When we call check_dependencies() or run() on the first Stage
# (system_checks), it will operate on the first Stage and then recursively
# call check_dependencies() or run() on the next_stage.
pre_ponr_changes = Stage("pre_ponr_changes", "Making recoverable changes")
system_checks = Stage("system_checks", "Check whether system is ready for conversion", next_stage=pre_ponr_changes)
try:
# Check dependencies are satisfied for system_checks and all subsequent
# Stages.
system_checks.check_dependencies()
except DependencyError as e:
# We want to fail early if dependencies are not properly set. This
# way we should fail in testing before release.
logger.critical("Some dependencies were set on Actions but not present in convert2rhel: %s" % e)
# Run the Actions in system_checks and all subsequent Stages.
results = system_checks.run()
# Format results as a dictionary:
# {"$Action_id": {"message_level": int,
# "message_id": "$message_id",
# "message": "" or "$message"},
# }
formatted_results = {}
for action in itertools.chain(*results):
# formatted_results[action.id] = action.result.to_dict()
formatted_results[action.id] = {"result": action.result.to_dict(), "messages": []}
for message in action.messages:
formatted_results[action.id]["messages"].append(message.to_dict())
return formatted_results
def find_actions_of_severity(results, severity):
"""
Filter results from :func:`run_actions` to include only results of ``severity`` or higher. # E: line too long (94 > 79 characters)
:param results: Results dictionary as returned by :func:`run_actions`
:type results: Mapping
:param severity: The name of a ``STATUS_CODE`` for the severity to filter to. # E: line too long (81 > 79 characters)
:returns: List of actions which are at ``severity`` or higher result. Empty list # E: line too long (84 > 79 characters)
if there were no failures.
:rtype: Sequence
Example::
matched_actions = find_actions_of_severity(results, "SKIP")
# matched_actions will contain all actions which were skipped
# or failed while running.
"""
matched_actions = [action for action in results.items() if action[1]["message_level"] >= STATUS_CODE[severity]] # E: line too long (108 > 79 characters)
# (when you do the items in FUTURE) matched_actions = [action for action in results.items() if action[1]["result"]["message_level"] >= STATUS_CODE[severity]]
# matched_actions will be a list of tuples
# [ (action.id1, {message-level, message-id, message}), ..]
return matched_actions
```
## Future
1. Change action.run_actions() to format the results differently:
```
{"$action_id": {"result": {"message_level": int,
"message_id": None or "$message_id",
"message": "" or "$message"},
"messages": [
{"message_level": int,
"message_id": "$message_id",
"message": "" or "$message",
},
# ...
]
},
},
}
```
(Example with some data: https://toshio.fedorapeople.org/convert2rhel/c2r-assessment-mockup.log )
2. Modify report.summary to output warnings and other messages in addition to the final results.
``` python
# result["status"] -> result["result"]["message_level"]
# add for loop to deal with result["messages"], nested in for loop and dealt with in a similiar fashion
# report.summary
#
logger.task("Pre-conversion analysis report")
combined_results_and_message = []
for action_id, action_value in results.items():
combined_results_and_message.append((action_id, action_value["result"]))
for message in action_value["messages"]:
combined_results_and_message.append((action_id, message))
# Operate on combined_results_and_message instead of results from here on
if include_all_reports:
results = results.items() #handled on 299
else:
results = find_actions_of_severity(results, "WARNING") #look at areas where this is used and need changes
#anywhere that uses results needs to use combined_results_and_message instead
terminal_size = utils.get_terminal_size()
word_wrapper = textwrap.TextWrapper(subsequent_indent=" ", width=terminal_size[0], replace_whitespace=False)
# Sort the results in reverse order, this way, the most important messages
# will be on top.
results = sorted(results, key=lambda item: item[1]["status"], reverse=True)
report = []
last_status = ""
for action_id, result in results:
if last_status != result["status"]:
report.append("")
report.append(format_report_section_heading(result["status"]))
last_status = result["status"]
entry = format_action_status_message(result["status"], action_id, result["error_id"], result["message"]) # E: line too long (112 > 79 characters)
entry = word_wrapper.fill(entry)
if with_colors:
entry = colorize(entry, _STATUS_TO_COLOR[result["status"]])
report.append(entry)
# If there is no other message sent to the user, then we just give a
# happy message to them.
if not results:
report.append("No problems detected during the analysis!")
logger.info("%s\n" % "\n".join(report))
```