Skip to content

Commit

Permalink
vdk-core: execution result missing exception and blamee fix (#938)
Browse files Browse the repository at this point in the history
* vdk-core: execution result missing exception and blamee fix

The execution result was sometimes and sometimes not provisioned with
the exception in case such is known. It was also lacking information
about blamee, yet interfacing unclassified (raw) exceptions.
Therefore, the information about error classification was not present
in ExecutionResult, only in StepResult (design flaw).
Also, data job execution summary was not referring to the exception
dynamically (via getter), that was prone to non-initialized exception.

Improved the `ExecutionResult` design by elaborating on blamee alongside
a raw-or-not-raw exception. Fixed the rest of the ExecutionResult
initialization implementation so that in all the cases the exception is
known, then it is populated.
Refined the data job execution summary by adding exception_name
and blamee in case of job failure.

Testing Done: Test-fixed expectation about a non-populated exception
in ExecutionResult, and added blamee check.
Updated tests to using `get_exception_to_raise()` in favour of the
deprecated `get_exception()`.

Signed-off-by: ikoleva <[email protected]>
  • Loading branch information
ivakoleva authored Aug 11, 2022
1 parent 44ef4d1 commit ee4b930
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def run_job(context: JobContext) -> ExecutionResult:
"""
start_time = datetime.utcnow()
exception = None
blamee = None
steps = context.step_builder.get_steps()
step_results = []

Expand All @@ -133,6 +134,7 @@ def run_job(context: JobContext) -> ExecutionResult:
)
except BaseException as e:
blamee = whom_to_blame(e, __file__, context.job_directory)
exception = e
errors.log_exception(
blamee,
log,
Expand Down Expand Up @@ -167,8 +169,9 @@ def run_job(context: JobContext) -> ExecutionResult:
start_time,
datetime.utcnow(),
execution_status,
exception,
step_results,
exception,
blamee,
)
return execution_result

Expand Down Expand Up @@ -306,8 +309,9 @@ def run(self, args: dict = None) -> ExecutionResult:
start_time,
datetime.utcnow(),
ExecutionStatus.ERROR,
ex,
[],
ex,
blamee,
)
return execution_result

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
from typing import Optional

from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus
from vdk.internal.core import errors
from vdk.internal.core.errors import ErrorMessage
from vdk.internal.core.errors import find_whom_to_blame_from_exception
from vdk.internal.core.errors import PlatformServiceError
from vdk.internal.core.errors import ResolvableBy


@dataclass(frozen=True)
Expand All @@ -34,7 +35,7 @@ class StepResult:
# Exception if thrown
exception: Optional[BaseException] = None
# who is responsible for resolving the error
blamee: Optional[errors.ResolvableBy] = None
blamee: Optional[ResolvableBy] = None


class ExecutionResult:
Expand All @@ -49,8 +50,9 @@ def __init__(
start_time: datetime,
end_time: datetime,
status: ExecutionStatus,
exception: Optional[BaseException],
steps_list: List[StepResult],
exception: Optional[BaseException],
blamee: Optional[ResolvableBy],
):
self.data_job_name = data_job_name
self.execution_id = execution_id
Expand All @@ -59,6 +61,7 @@ def __init__(
self.status = status
self.steps_list = steps_list
self.exception = exception
self.blamee = blamee

def is_failed(self):
return self.status == ExecutionStatus.ERROR
Expand Down Expand Up @@ -94,13 +97,23 @@ def get_exception_to_raise(self):
)
)

def get_blamee(self) -> Optional[ResolvableBy]:
if self.blamee:
return self.blamee
exception = self.get_exception_to_raise()

step_raising_exception = next(
filter(lambda s: s.exception == exception, self.steps_list)
)
if step_raising_exception:
return step_raising_exception.blamee
return find_whom_to_blame_from_exception(exception)

def __repr__(self):
data = self.__dict__.copy()
# make dates more human-readble
data["start_time"] = self.start_time.isoformat()
data["end_time"] = self.end_time.isoformat()
if self.exception:
data["exception_name"] = str(self.exception.__class__.__name__)

step_lists_of_dicts = list()
for step in self.steps_list:
Expand All @@ -124,6 +137,12 @@ def __repr__(self):

data["steps_list"] = step_lists_of_dicts

if self.is_failed():
data["exception_name"] = str(
self.get_exception_to_raise().__class__.__name__
)
data["blamee"] = self.get_blamee()

def default_serialization(o: Any) -> Any:
return o.__dict__ if "__dict__" in dir(o) else str(o)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from vdk.internal.builtin_plugins.run.execution_results import ExecutionResult
from vdk.internal.builtin_plugins.run.job_context import JobContext
from vdk.internal.builtin_plugins.run.step import Step
from vdk.internal.core.errors import ResolvableBy
from vdk.plugin.test_utils.util_funcs import DataJobBuilder


Expand All @@ -23,15 +24,17 @@ def test_run_when_step_fails():

result = data_job.run()
assert result.is_failed()
assert result.exception is None
assert isinstance(result.get_exception(), IndentationError)
assert isinstance(result.get_exception_to_raise(), IndentationError)
assert result.get_blamee() == ResolvableBy.USER_ERROR


def test_run_when_step_succeeds():
job_builder = DataJobBuilder()
job_builder.add_step_func(lambda s, i: True)
result = job_builder.build().run()
assert result.is_success()
assert result.exception is None
assert result.blamee is None


def test_run_job_with_default_hook():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from vdk.internal.builtin_plugins.run.execution_results import StepResult
from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus
from vdk.internal.core import errors
from vdk.internal.core.errors import ResolvableBy


class NonJsonSerializable:
Expand All @@ -33,8 +34,9 @@ def test_serialization():
datetime.fromisoformat("2012-10-12 00:00:00"),
datetime.fromisoformat("2012-10-12 01:00:00"),
ExecutionStatus.SUCCESS,
None,
[],
None,
None,
)

assert (
Expand All @@ -46,7 +48,8 @@ def test_serialization():
"end_time": "2012-10-12T01:00:00",
"status": "success",
"steps_list": [],
"exception": null
"exception": null,
"blamee": null
}"""
)

Expand All @@ -64,7 +67,7 @@ def test_get_exception_to_raise_main_error():
assert result.get_exception_to_raise() == error


def _prepare_execution_result(error, step_error):
def _prepare_execution_result(error, step_error, blamee=None):
step_result = StepResult(
"step",
"type",
Expand All @@ -81,8 +84,9 @@ def _prepare_execution_result(error, step_error):
datetime.fromisoformat("2012-10-12 00:00:00"),
datetime.fromisoformat("2012-10-12 01:00:00"),
ExecutionStatus.SUCCESS,
error,
[step_result],
error,
blamee,
)
return result

Expand All @@ -97,15 +101,17 @@ def test_serialization_non_serializable():
ExecutionStatus.ERROR,
"details",
exception,
ResolvableBy.USER_ERROR,
)
result = ExecutionResult(
"job-name",
"exec-id",
datetime.fromisoformat("2012-10-12 00:00:00"),
datetime.fromisoformat("2012-10-12 01:00:00"),
ExecutionStatus.ERROR,
exception,
[step_result],
exception,
ResolvableBy.USER_ERROR,
)

result_as_string = result.__repr__()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus
from vdk.internal.builtin_plugins.templates.template_impl import TemplatesImpl
from vdk.internal.core.context import CoreContext
from vdk.internal.core.errors import ResolvableBy
from vdk.internal.core.errors import UserCodeError


Expand All @@ -36,7 +37,7 @@ def test_template_execute():
mock_job_factory = MagicMock(spec=DataJobFactory)
mock_job = MagicMock(spec=DataJob)
mock_job.run.return_value = ExecutionResult(
"foo", "1", None, None, ExecutionStatus.SUCCESS, None, []
"foo", "1", None, None, ExecutionStatus.SUCCESS, [], None, None
)
mock_job_factory.new_datajob.return_value = mock_job
mock_context = MagicMock(spec=CoreContext)
Expand All @@ -54,7 +55,14 @@ def test_template_execute_template_fails_raise_exception():
mock_job_factory = MagicMock(spec=DataJobFactory)
mock_job = MagicMock(spec=DataJob)
mock_job.run.return_value = ExecutionResult(
"foo", "1", None, None, ExecutionStatus.ERROR, AttributeError("dummy"), []
"foo",
"1",
None,
None,
ExecutionStatus.ERROR,
[],
AttributeError("dummy"),
ResolvableBy.USER_ERROR,
)
mock_job_factory.new_datajob.return_value = mock_job
mock_context = MagicMock(spec=CoreContext)
Expand Down

0 comments on commit ee4b930

Please sign in to comment.