From ee4b93022a5aae3346ff3ebce2d4ed2dbe8ba82c Mon Sep 17 00:00:00 2001 From: ivakoleva Date: Thu, 11 Aug 2022 18:14:01 +0300 Subject: [PATCH] vdk-core: execution result missing exception and blamee fix (#938) * 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 --- .../internal/builtin_plugins/run/data_job.py | 8 +++-- .../builtin_plugins/run/execution_results.py | 29 +++++++++++++++---- .../builtin_plugins/run/test_data_job.py | 7 +++-- .../run/test_execution_results.py | 16 ++++++---- .../templates/test_template_impl.py | 12 ++++++-- 5 files changed, 56 insertions(+), 16 deletions(-) diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py index 257753cb89..a14bd52771 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py @@ -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 = [] @@ -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, @@ -167,8 +169,9 @@ def run_job(context: JobContext) -> ExecutionResult: start_time, datetime.utcnow(), execution_status, - exception, step_results, + exception, + blamee, ) return execution_result @@ -306,8 +309,9 @@ def run(self, args: dict = None) -> ExecutionResult: start_time, datetime.utcnow(), ExecutionStatus.ERROR, - ex, [], + ex, + blamee, ) return execution_result diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/execution_results.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/execution_results.py index ffce980b5c..d2d311fd0a 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/execution_results.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/execution_results.py @@ -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) @@ -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: @@ -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 @@ -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 @@ -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: @@ -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) diff --git a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_data_job.py b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_data_job.py index 0ea28dcc19..2848f68dd2 100644 --- a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_data_job.py +++ b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_data_job.py @@ -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 @@ -23,8 +24,8 @@ 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(): @@ -32,6 +33,8 @@ def test_run_when_step_succeeds(): 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(): diff --git a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_execution_results.py b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_execution_results.py index daf1d86fa3..a9b8dabe9e 100644 --- a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_execution_results.py +++ b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/test_execution_results.py @@ -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: @@ -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 ( @@ -46,7 +48,8 @@ def test_serialization(): "end_time": "2012-10-12T01:00:00", "status": "success", "steps_list": [], - "exception": null + "exception": null, + "blamee": null }""" ) @@ -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", @@ -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 @@ -97,6 +101,7 @@ def test_serialization_non_serializable(): ExecutionStatus.ERROR, "details", exception, + ResolvableBy.USER_ERROR, ) result = ExecutionResult( "job-name", @@ -104,8 +109,9 @@ def test_serialization_non_serializable(): 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__() diff --git a/projects/vdk-core/tests/vdk/internal/builtin_plugins/templates/test_template_impl.py b/projects/vdk-core/tests/vdk/internal/builtin_plugins/templates/test_template_impl.py index d59d975d8a..3868a977d9 100644 --- a/projects/vdk-core/tests/vdk/internal/builtin_plugins/templates/test_template_impl.py +++ b/projects/vdk-core/tests/vdk/internal/builtin_plugins/templates/test_template_impl.py @@ -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 @@ -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) @@ -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)