#Design for abort actions from a stop step with error option enabled

1 messages · Page 1 of 1 (latest)

wraith prawn
#

Basically, the idea is to add a boolean field for _AbortScript exceptions telling if the exception must be propaged or not.

#

The changes are pretty straightforward. Check below

#

Creation of a init method in the abort script to set the propagate_abortion variable

class _AbortScript(_HaltScript):
    """Throw if script needs to abort because of an unexpected error."""
    
    def __init__(self, message: str, propagate_abortion: bool = False):
        super().__init__(message)
        self.propagate_abortion = propagate_abortion
#

Adding the propagate_abortion value whenever the error option is true:

    async def _async_stop_step(self):
        """Stop script execution."""
        stop = self._action[CONF_STOP]
        error = self._action.get(CONF_ERROR, False)
        trace_set_result(stop=stop, error=error)
        if error:
            self._log("Error script sequence: %s", stop)
            raise _AbortScript(message=stop, propagate_abortion=True)
#

Checking for the propagate_abortion in the script caller (async_run):

        try:
            self._log("Running %s", self._script.running_description)
            for self._step, self._action in enumerate(self._script.sequence):
                if self._stop.is_set():
                    script_execution_set("cancelled")
                    break
                await self._async_step(log_exceptions=False)
            else:
                script_execution_set("finished")
        except _AbortScript as err:
            script_execution_set("aborted")
            # Let the _AbortScript bubble up if this is a sub-script or if it comes 
            # from a stop step with error option enabled
            if not self._script.top_level or err.propagate_abortion:
                raise
#

BTW, if you noticed, when the error option is False, the Stop step is raising _StopScript

#

At the end of the day, without my change, raising the _StopScript or _AbortScript has the same behavior (except for the possibility of having a response in the _StopScript), which doesn't make much sense since they are different exceptions.

#

Look at the code without my change:

# 
        except _AbortScript as err:
            script_execution_set("aborted")
            # Let the _AbortScript bubble up if this is a sub-script or if it comes from 
            # a stop step with error option enabled
            if not self._script.top_level:
                raise

        except _ConditionFail:
            script_execution_set("aborted")
        except _StopScript as err:
            script_execution_set("finished", err.response)
            response = err.response

            # Let the _StopScript bubble up if this is a sub-script
            if not self._script.top_level:
                # We already consumed the response, do not pass it on
                err.response = None
                raise err
# 
    async def _async_stop_step(self):
        """Stop script execution."""
        stop = self._action[CONF_STOP]
        error = self._action.get(CONF_ERROR, False)
        trace_set_result(stop=stop, error=error)
        if error:
            self._log("Error script sequence: %s", stop)
            raise _AbortScript(stop)

        self._log("Stop script sequence: %s", stop)
        if CONF_RESPONSE_VARIABLE in self._action:
            try:
                response = self._variables[self._action[CONF_RESPONSE_VARIABLE]]
            except KeyError as ex:
                raise _AbortScript(
                    f"Response variable '{self._action[CONF_RESPONSE_VARIABLE]}' "
                    "is not defined"
                ) from ex
        else:
            response = None
        raise _StopScript(stop, response)
#

It's pretty much the same behavior. My changes are, in fact, causing the error option to stop all the sequences from its script and a parent one because something went wrong.

#

But I don't know if this is the best approach, this thread is to discuss it.

formal rampart
#

sorry I know this is not the point of this, but I don't think propagate_abortion is a good name for what this is

#

as for the implementation, the point of discussing here is to find a path to an implementation. Since you think you have one, I'd PR it and then you'll get better feedback on whether it's the right approach or not because the people reviewing are probably more familiar with this code than the majority of the people in this channel

wraith prawn
#

You don’t have to be sorry, it’s okay! Sometimes I’m not good with names, but I would like to learn, what is the problem for you with “propagate_abortion”?

#

I’m going to get a better name and create the PR

formal rampart
#

@wraith prawn abortion is a hot button topic in the United States. My mind immediately went there when I read this