Add BrigadeSubTaskError for grouped tasks #131

ogenstad · 2018-05-02T17:40:10Z

Raises BrigadeSubTaskError when a subtask to a grouped task has failed,
this allows for a prettier error message when using print_output()

I haven't changed the output from the tests for print_output yet so currently this will fail.

With a test script:

from brigade.core import InitBrigade
from brigade.plugins.functions.text import print_result
from brigade.core.task import Result


brg = InitBrigade(
    raise_on_error=False)


def greet(task, msg):
    return Result(host=task.host, result="{}".format(msg))


def fail_on_me(task):
    with open('does_not_exist.txt', 'r') as fs:
        data = fs.read()
    return Result(host=task.host, result=data)


def sub_task(task):
    task.run(task=greet, msg="first")
    task.run(task=greet, msg="second")
    task.run(task=fail_on_me)
    task.run(task=greet, msg="third")
    return Result(host=task.host, result="grouped_task")


result = brg.run(sub_task)
print_result(result)

Running the script with the current develop branch:

sub_task************************************************************************
* srv1 ** changed : False ******************************************************
vvvv sub_task ** changed : False vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv ERROR
Traceback (most recent call last):
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 61, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 23, in sub_task
    task.run(task=fail_on_me)
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 98, in run
    raise r.exception
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 61, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 15, in fail_on_me
    with open('does_not_exist.txt', 'r') as fs:
FileNotFoundError: [Errno 2] No such file or directory: 'does_not_exist.txt'

---- greet ** changed : False -------------------------------------------------- INFO
first
---- greet ** changed : False -------------------------------------------------- INFO
second
^^^^ END sub_task ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* srv2 ** changed : False ******************************************************
vvvv sub_task ** changed : False vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv ERROR
Traceback (most recent call last):
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 61, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 23, in sub_task
    task.run(task=fail_on_me)
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 98, in run
    raise r.exception
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 61, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 15, in fail_on_me
    with open('does_not_exist.txt', 'r') as fs:
FileNotFoundError: [Errno 2] No such file or directory: 'does_not_exist.txt'

---- greet ** changed : False -------------------------------------------------- INFO
first
---- greet ** changed : False -------------------------------------------------- INFO
second
^^^^ END sub_task ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Running the script with this PR:

sub_task************************************************************************
* srv1 ** changed : False ******************************************************
vvvv sub_task ** changed : False vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv ERROR

########################################
# Subtask: fail_on_me (failed)
########################################

---- greet ** changed : False -------------------------------------------------- INFO
first
---- greet ** changed : False -------------------------------------------------- INFO
second
---- fail_on_me ** changed : False --------------------------------------------- ERROR
Traceback (most recent call last):
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 62, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 15, in fail_on_me
    with open('does_not_exist.txt', 'r') as fs:
FileNotFoundError: [Errno 2] No such file or directory: 'does_not_exist.txt'

^^^^ END sub_task ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* srv2 ** changed : False ******************************************************
vvvv sub_task ** changed : False vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv ERROR

########################################
# Subtask: fail_on_me (failed)
########################################

---- greet ** changed : False -------------------------------------------------- INFO
first
---- greet ** changed : False -------------------------------------------------- INFO
second
---- fail_on_me ** changed : False --------------------------------------------- ERROR
Traceback (most recent call last):
  File "/Users/patrick/src/brigade/brigade/core/task.py", line 62, in start
    r = self.task(self, **self.params)
  File "will-fail.py", line 15, in fail_on_me
    with open('does_not_exist.txt', 'r') as fs:
FileNotFoundError: [Errno 2] No such file or directory: 'does_not_exist.txt'

^^^^ END sub_task ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

With something like this in place I think we can also ignore #129.

Raises BrigadeSubTaskError when a subtask to a grouped task has failed, this allows for a prettier error message when using print_output()

coveralls · 2018-05-02T17:47:32Z

Pull Request Test Coverage Report for Build 647

15 of 15 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 93.053%

Totals
Change from base Build 624:	0.2%
Covered Lines:	951
Relevant Lines:	1022

💛 - Coveralls

dbarrosop · 2018-05-03T08:11:05Z

brigade/core/exceptions.py

+    has failed
+
+    Arguments:
+        task (str): Name of the sub task that failed


I think task should be a reference the task itself, not just the name. I think we should also have a reference to the result. So you'd basically call this like (see comment below)

dbarrosop · 2018-05-03T08:12:51Z

brigade/core/task.py

        if r.failed:
            # Without this we will keep running the grouped task
-            raise r.exception
+            raise BrigadeSubTaskError((r.name))


Given my previous comment I think this should be:

raise BrigadeSubTaskError(task=task, error=r)

That will also mean we have to change:

r = Task(task, **kwargs).start(self.host, self.brigade)

to:

task = Task(task, **kwargs) r = task.start(self.host, self.brigade)

dbarrosop · 2018-05-03T17:21:14Z

brigade/core/exceptions.py

+    def __str__(self):
+        text = "\n"
+        text += "{}\n".format("#" * 40)
+        text += "# Subtask: {} (failed)\n".format(self.task)


I'd remove the # (including the space before the S)

Sorry, just reread it. I think we should just return Subtask: {} (failed)\n".format(self.task). Nothing else.

dbarrosop · 2018-05-04T07:44:47Z

brigade/core/task.py

@@ -61,10 +62,17 @@ def start(self, host, brigade):
            r = self.task(self, **self.params)
            if not isinstance(r, Result):
                r = Result(host=host, result=r)
+
+        except BrigadeSubTaskError as e:


I don't think we need this block, do we?

There's a small difference as I don't save the traceback in the result key after BrigadeSubTaskError. Do you want to remove it?

dbarrosop · 2018-05-04T07:46:35Z

tests/plugins/functions/text/output_data/failed_with_severity.stdout

@@ -13,7 +13,9 @@ Hello from CRITICAL
 [1m[32m^^^^ END read_data ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 [1m[34m* dev4.group_2 ** changed : False **********************************************
 [1m[31mvvvv read_data ** changed : False vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv ERROR
-Exception('Unknown Error -> Contact your system administrator',)
+BrigadeSubTaskError()


Shouldn't this have been something like Subtask: parse_data (failed)\n?

It's due to this:
https://github.com/brigade-automation/brigade/blob/develop/tests/plugins/functions/text/test_print_result.py#L101

Because the result key isn't printed. That's to avoid showing the traceback which contains my local paths.

I think that should output the task name and the arguments used if possible. If it's too hard we can open an issue and fix it later

Add BrigadeSubTaskError for grouped tasks

4ce01ef

Raises BrigadeSubTaskError when a subtask to a grouped task has failed, this allows for a prettier error message when using print_output()

dbarrosop reviewed May 3, 2018

View reviewed changes

ogenstad added 2 commits May 3, 2018 18:58

Use objects intead of name of task

6f2ed5f

Black

42d289c

dbarrosop reviewed May 3, 2018

View reviewed changes

Clean up __str__

44b383b

ogenstad changed the title ~~For discussion: Add BrigadeSubTaskError for grouped tasks~~ Add BrigadeSubTaskError for grouped tasks May 3, 2018

Version 1.0.0

87d4b94

dbarrosop reviewed May 4, 2018

View reviewed changes

dbarrosop approved these changes May 4, 2018

View reviewed changes

ogenstad merged commit 28a57e0 into develop May 4, 2018

ogenstad deleted the sub_task_exceptions branch May 4, 2018 08:29

ogenstad mentioned this pull request May 5, 2018

Revisit output from Subtask Errors #134

Closed

ogenstad mentioned this pull request May 23, 2018

Exception doesn't exist for self defined failures #129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BrigadeSubTaskError for grouped tasks #131

Add BrigadeSubTaskError for grouped tasks #131

ogenstad commented May 2, 2018

coveralls commented May 2, 2018 •

edited

Loading

dbarrosop May 3, 2018

dbarrosop May 3, 2018

dbarrosop May 3, 2018 •

edited

Loading

dbarrosop May 3, 2018

dbarrosop May 4, 2018

ogenstad May 4, 2018

dbarrosop May 4, 2018

ogenstad May 4, 2018

dbarrosop May 4, 2018

Add BrigadeSubTaskError for grouped tasks #131

Add BrigadeSubTaskError for grouped tasks #131

Conversation

ogenstad commented May 2, 2018

coveralls commented May 2, 2018 • edited Loading

Pull Request Test Coverage Report for Build 647

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbarrosop May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 2, 2018 •

edited

Loading

dbarrosop May 3, 2018 •

edited

Loading