vdk-core: replace report_and_rethrow

Why?

Calling report_and_rethrow causes confusing stack traces,
because of adding an extra frame from the extra function call

What?

Replace report_and_rethrow with just calling report and raising
the exception after

How was this tested?

CI

What kind of change is this?

Feature/non-breaking

Signed-off-by: Dilyan Marinov <[email protected]>

projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_connection_base.py

@@ -156,10 +156,8 @@ def execute_query(self, query: str) -> List[List[Any]]:
                             ]
                         )
                     )
-                    errors.report_and_rethrow(
-                        blamee,
-                        e,
-                    )
+                    errors.report(blamee, e)
+                    raise e
             return cast(
                 List[List[Any]], res
             )  # we return None in case of DML. This is not PEP249 compliant, but is more convenient

projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_cursor.py

@@ -95,7 +95,8 @@ def execute(
         try:
             result = self._execute_operation(managed_operation)
             self._log.info(
-                f"Executing query SUCCEEDED. Query {_get_query_duration(query_start_time)}"
+                f"Executing query SUCCEEDED. Query {
+                    _get_query_duration(query_start_time)}"
             )
             return result
         except Exception as e:
@@ -110,7 +111,8 @@ def execute(
                 blamee = errors.ResolvableBy.USER_ERROR
                 # else:
                 #     blamee = errors.ResolvableBy.PLATFORM_ERROR
-                self._log.info(f"Failed query {_get_query_duration(query_start_time)}")
+                self._log.info(f"Failed query {
+                               _get_query_duration(query_start_time)}")
                 self._log.error(
                     "\n".join(
                         [
@@ -119,10 +121,11 @@ def execute(
                         ]
                     )
                 )
-                errors.report_and_rethrow(
+                errors.report(
                     blamee,
                     e,
                 )
+                raise e
 
     def _decorate_operation(self, managed_operation: ManagedOperation, operation: str):
         if self.__connection_hook_spec.db_connection_decorate_operation.get_hookimpls():
@@ -143,7 +146,9 @@ def _decorate_operation(self, managed_operation: ManagedOperation, operation: st
                         ]
                     )
                 )
-                errors.report_and_rethrow(errors.ResolvableBy.PLATFORM_ERROR, e)
+                errors.report(
+                    errors.ResolvableBy.PLATFORM_ERROR, e)
+                raise e
 
     def _validate_operation(self, operation: str, parameters: Optional[Container]):
         if self.__connection_hook_spec.db_connection_validate_operation.get_hookimpls():
@@ -161,14 +166,17 @@ def _validate_operation(self, operation: str, parameters: Optional[Container]):
                         ]
                     )
                 )
-                errors.report_and_rethrow(
+                errors.report(
                     errors.ResolvableBy.USER_ERROR,
                     exception=e,
                 )
+                raise e
 
     def _execute_operation(self, managed_operation: ManagedOperation):
-        self._log.info("Executing query:\n%s" % managed_operation.get_operation())
-        execution_cursor = ExecutionCursor(self._cursor, managed_operation, self._log)
+        self._log.info("Executing query:\n%s" %
+                       managed_operation.get_operation())
+        execution_cursor = ExecutionCursor(
+            self._cursor, managed_operation, self._log)
         if self.__connection_hook_spec:
             result = self.__connection_hook_spec.db_connection_execute_operation(
                 execution_cursor=execution_cursor
@@ -213,7 +221,8 @@ def _recover_operation(self, exception, managed_operation):
             managed_operation,
             self.__connection_hook_spec.db_connection_decorate_operation,
         )
-        self._log.debug(f"Recovery of query {managed_operation.get_operation()}")
+        self._log.debug(f"Recovery of query {
+                        managed_operation.get_operation()}")
         try:
             self.__connection_hook_spec.db_connection_recover_operation(
                 recovery_cursor=recovery_cursor
@@ -234,7 +243,8 @@ def _recover_operation(self, exception, managed_operation):
 
 def _get_query_duration(query_start_time: float):
     query_end_time = timer()
-    seconds = timedelta(seconds=query_end_time - query_start_time).total_seconds()
+    seconds = timedelta(seconds=query_end_time -
+                        query_start_time).total_seconds()
     minutes, seconds = divmod(seconds, 60)
     hours, minutes = divmod(minutes, 60)
     difference = f"{int(hours):02}h:{int(minutes):02}m:{int(seconds):02}s"

projects/vdk-core/src/vdk/internal/builtin_plugins/run/cli_run.py

@@ -49,7 +49,8 @@ def __validate_and_parse_args(arguments: str) -> Optional[Dict]:
                     ]
                 )
             )
-            errors.report_and_rethrow(errors.ResolvableBy.USER_ERROR, e)
+            errors.report(errors.ResolvableBy.USER_ERROR, e)
+            raise e
 
     @staticmethod
     def __split_into_chunks(exec_steps: List, chunks: int) -> List:
@@ -66,27 +67,30 @@ def __split_into_chunks(exec_steps: List, chunks: int) -> List:
                 i if i < remainder else remainder
             ) + quotient * (0 if i < remainder else i - remainder)
             yield exec_steps[
-                subsequent_iteration : subsequent_iteration
+                subsequent_iteration: subsequent_iteration
                 + (quotient + 1 if i < remainder else quotient)
             ]
 
     @staticmethod
     def __warn_on_python_version_disparity(
         context: CoreContext, job_directory: pathlib.Path
     ):
-        log_config_type = context.configuration.get_value(vdk_config.LOG_CONFIG)
+        log_config_type = context.configuration.get_value(
+            vdk_config.LOG_CONFIG)
         if log_config_type == "LOCAL":
             # Get the local python installation's version.
             python_env_version = sys.version_info
-            local_py_version = f"{python_env_version.major}.{python_env_version.minor}"
+            local_py_version = f"{python_env_version.major}.{
+                python_env_version.minor}"
 
             # Get the python_version set in the config.ini if any.
             job_path = job_directory.resolve()
             try:
                 config = JobConfig(data_job_path=job_path)
             except VdkConfigurationError as e:
                 log.info(
-                    f"An exception occurred while loading job configuration. Error was {e}"
+                    f"An exception occurred while loading job configuration. Error was {
+                        e}"
                 )
                 return
             configured_python_version = config.get_python_version()
@@ -131,7 +135,8 @@ def __log_exec_result(self, execution_result: ExecutionResult) -> None:
             steps = temp_exec_result.pop("steps_list")
 
             log.info(
-                f"Data Job execution summary: {json.dumps(temp_exec_result, indent=2)}"
+                f"Data Job execution summary: {
+                    json.dumps(temp_exec_result, indent=2)}"
             )
 
             chunks = math.ceil(len(string_exec_result) / 5000)
@@ -163,7 +168,8 @@ def create_and_run_data_job(
         arguments: Optional[str],
     ):
         log.info(f"Run job with directory {data_job_directory}")
-        context.plugin_registry.load_plugin_with_hooks_impl(ExecutionTrackingPlugin())
+        context.plugin_registry.load_plugin_with_hooks_impl(
+            ExecutionTrackingPlugin())
 
         self.__warn_on_python_version_disparity(
             context=context, job_directory=data_job_directory
@@ -193,12 +199,13 @@ def create_and_run_data_job(
                     ]
                 )
             )
-            errors.report_and_rethrow(
+            errors.report(
                 job_input_error_classifier.whom_to_blame(
                     e, __file__, data_job_directory
                 ),
                 e,
             )
+            raise e
         if execution_result.is_failed() and execution_result.get_exception_to_raise():
             raise execution_result.get_exception_to_raise()
 
@@ -219,7 +226,8 @@ def create_and_run_data_job(
 )
 @click.argument(
     "data_job_directory",
-    type=click.Path(exists=True, file_okay=False, dir_okay=True, resolve_path=True),
+    type=click.Path(exists=True, file_okay=False,
+                    dir_okay=True, resolve_path=True),
 )
 @click.option(
     "--arguments",
@@ -238,7 +246,8 @@ def run(ctx: click.Context, data_job_directory: str, arguments: str) -> None:
     Entry point of the CLI run. It start a run (execution) of a data job.
     """
     log.info(
-        f"Versatile Data Kit (VDK){os.linesep}{version.get_version_info()}{os.linesep + '-' * 80}"
+        f"Versatile Data Kit (VDK){os.linesep}{version.get_version_info()}{
+            os.linesep + '-' * 80}"
     )
     context: CoreContext = cast(CoreContext, ctx.obj)
     run_impl = CliRunImpl()

projects/vdk-core/src/vdk/internal/builtin_plugins/run/file_based_step.py

@@ -97,21 +97,24 @@ def run(job_input)
                         ]
                     )
                 )
-                errors.report_and_rethrow(errors.ResolvableBy.USER_ERROR, e)
+                errors.report(errors.ResolvableBy.USER_ERROR, e)
+                raise e
 
             for _, func in inspect.getmembers(python_module, inspect.isfunction):
                 if func.__name__ == "run":
                     try:
                         log.info("Entering %s#run(...) ..." % filename)
-                        StepFuncFactory.invoke_run_function(func, job_input, step.name)
+                        StepFuncFactory.invoke_run_function(
+                            func, job_input, step.name)
                         success = True
                         return True
                     finally:
                         if success:
                             log.info("Exiting  %s#run(...) SUCCESS" % filename)
                             errors.resolvable_context().mark_all_resolved()
                         else:
-                            log.error("Exiting  %s#run(...) FAILURE" % filename)
+                            log.error("Exiting  %s#run(...) FAILURE" %
+                                      filename)
             log.warning(
                 "File %s does not contain a valid run() method. Nothing to execute. Skipping %s,"
                 + " and continuing with other files (if present).",
@@ -153,7 +156,8 @@ def invoke_run_function(func: Callable, job_input: IJobInput, step_name: str):
                 )
 
                 to_be_fixed_by = whom_to_blame(e, __file__, None)
-                errors.report_and_rethrow(to_be_fixed_by, e)
+                errors.report(to_be_fixed_by, e)
+                raise e
         else:
             errors.report_and_throw(
                 errors.UserCodeError(
@@ -162,6 +166,7 @@ def invoke_run_function(func: Callable, job_input: IJobInput, step_name: str):
                     "Current Step (python file) will fail, and as a result the whole Data Job will fail. ",
                     "Make sure that you have specified a job input parameter in the signature of the "
                     "run method. "
-                    f"Possible parameters of run function are: {list(possible_arguments.keys())}.",
+                    f"Possible parameters of run function are: {
+                        list(possible_arguments.keys())}.",
                 )
             )

projects/vdk-core/src/vdk/internal/core/errors.py

@@ -87,17 +87,19 @@ def pretty_vdk_error_formatter(ex: BaseVdkError):
                 if br == 0:
                     br = max_len
                 wrapped_lines.append(l[: br + 1])
-                l = l[br + 1 :]
+                l = l[br + 1:]
             wrapped_lines.append(l)
         else:
             wrapped_lines.append(line)
     # build th header
     header = box_char + header.center(max_len + 6) + box_char
     # build the lines with the box char
-    lines = [box_char + "  " + s.ljust(max_len + 4) + box_char for s in wrapped_lines]
+    lines = [box_char + "  " +
+             s.ljust(max_len + 4) + box_char for s in wrapped_lines]
     # build the box sides
     side = (max_len + 8) * box_char
-    ex._pretty_message = "\n".join(["\n" + side, header, side, "\n".join(lines), side])
+    ex._pretty_message = "\n".join(
+        ["\n" + side, header, side, "\n".join(lines), side])
 
 
 class BaseVdkError(Exception):
@@ -116,7 +118,8 @@ def __init__(
         # Check if error message or dict was passed
         # for compatibility with vdk plugins
         self._lines = []
-        header = f"{self.__class__.__name__}: An error of resolvable type {vdk_resolvable_actual} occurred"
+        header = f"{self.__class__.__name__}: An error of resolvable type {
+            vdk_resolvable_actual} occurred"
         self._lines.append(header)
         if error_message_lines and isinstance(error_message_lines[0], ErrorMessage):
             message = error_message_lines[0]
@@ -259,7 +262,8 @@ def log_and_rethrow(
         # wrap
     message = [what_happened, why_it_happened, consequences, countermeasures]
     log.error("\n".join(message))
-    report_and_rethrow(to_be_fixed_by, exception)
+    report(to_be_fixed_by, exception)
+    raise exception
 
 
 class ErrorMessage:

projects/vdk-core/tests/vdk/internal/core/test_errors.py

@@ -168,6 +168,18 @@ def test_report_and_rethrow(self):
             is 1
         )
 
+    def test_report(self):
+        ex = IndexError("foo")
+        errors.report(
+            errors.ResolvableBy.USER_ERROR,
+            exception=ex,
+        )
+        assert errors.ResolvableByActual.USER in errors.resolvable_context().resolvables
+        assert (
+            len(errors.resolvable_context().resolvables[errors.ResolvableByActual.USER])
+            is 1
+        )
+
     def test_report_and_throw(self):
         with pytest.raises(errors.PlatformServiceError):
             errors.report_and_throw(PlatformServiceError("My super awesome message"))