apply review

Author: agoscinski

src/aiida/engine/processes/process.py

@@ -339,10 +339,6 @@ def kill(self, msg_text: str | None = None, force_kill: bool = False) -> Union[b
         """
         self.node.logger.info(f'Request to kill Process<{self.node.pk}>')
 
-        # PR_COMMENT Because we need to overwrite the logic of the cancelation of the self._killing task of the
-        #            scheduler job, we need to copy this logic of the parent class in plumpy, we need to adapt the
-        #            cancelation of the last sent killing action to also resend the kill/cancelation of the scheduler
-        #            job as we stop this canelation by canceling the last killing action
         if self.killed():
             # Already killed
             return True
@@ -356,11 +352,9 @@ def kill(self, msg_text: str | None = None, force_kill: bool = False) -> Union[b
             if self._killing:
                 self._killing.cancel()
 
-            # PR_COMMENT: We cannot reuse _killing because of type issues, it is a CancellableAction.
-            #             We can wrap a task around a CancellableAction but the CancellableAction catches silently any
-            #             error whilel here we need to know if the cancelation of the scheduler job failed.
             if self._cancelling_scheduler_job:
                 self._cancelling_scheduler_job.cancel()
+                self.node.logger.report('Found active scheduler job cancelation that will be rescheduled.')
 
             from .calcjobs.tasks import task_kill_job
 
@@ -369,7 +363,7 @@ def kill(self, msg_text: str | None = None, force_kill: bool = False) -> Union[b
             try:
                 self.loop.run_until_complete(self._cancelling_scheduler_job)
             except Exception as exc:
-                self.node.logger.error(f'While cancelling job error was raised: {exc!s}')
+                self.node.logger.error(f'While cancelling the scheduler job an error was raised: {exc!s}')
                 return False
 
         result = super().kill(msg_text, force_kill)
@@ -410,8 +404,6 @@ def done(done_future: plumpy.futures.Future):
 
         return result
 
-    # PR_COMMENT This is a copy of the function in engine/processes/calcjobs/tasks.py
-    #            and will merged to one place in PR #6868
     async def _launch_task(self, coro, *args, **kwargs):
         """Launch a coroutine as a task, making sure to make it interruptable."""
         import functools

src/aiida/engine/utils.py

@@ -193,7 +193,6 @@ async def exponential_backoff_retry(
     :param ignore_exceptions: exceptions to ignore, i.e. when caught do nothing and simply re-raise
     :return: result if the ``coro`` call completes within ``max_attempts`` retries without raising
     """
-
     if logger is None:
         logger = LOGGER
 

tests/cmdline/commands/test_process.py

@@ -26,7 +26,7 @@
 from aiida.engine import Process, ProcessState
 from aiida.engine.processes import control as process_control
 from aiida.engine.utils import exponential_backoff_retry
-from aiida.orm import CalcJobNode, Group, WorkChainNode, WorkflowNode, WorkFunctionNode
+from aiida.orm import CalcJobNode, Group, Int, WorkChainNode, WorkflowNode, WorkFunctionNode
 from tests.utils.processes import WaitProcess
 
 FuncArgs = tuple[t.Any, ...]
@@ -152,7 +152,6 @@ def test_process_kill_failing_transport(
     A failure in opening a transport connection results in the EBM to be fired blocking a regular kill command.
     The force kill command will ignore the EBM and kill the process in any case."""
     from aiida.cmdline.utils.common import get_process_function_report
-    from aiida.orm import Int
 
     code = aiida_code_installed(default_calc_job_plugin='core.arithmetic.add', filepath_executable='/bin/bash')
 
@@ -193,7 +192,6 @@ def test_process_kill_failing_transport_failed_kill(
     """
 
     from aiida.cmdline.utils.common import get_process_function_report
-    from aiida.orm import Int
 
     code = aiida_code_installed(default_calc_job_plugin='core.arithmetic.add', filepath_executable='/bin/bash')
 
@@ -235,8 +233,6 @@ def test_process_kill_failing_ebm_transport(
     It should be possible to kill it normally. A process that failed upload (e.g. in scenarios that transport is working
     again) and is then killed
     """
-    from aiida.orm import Int
-
     code = aiida_code_installed(default_calc_job_plugin='core.arithmetic.add', filepath_executable='/bin/bash')
 
     def make_a_builder(sleep_seconds=0):
@@ -275,7 +271,7 @@ def test_process_kill_failing_ebm_kill(
     Killing a process tries to gracefully cancel the job on the remote node. If there are connection problems it retries
     it in using the EBM. If this fails another kill command can be send to restart the cancelation of the job scheduler.
     """
-    from aiida.orm import Int
+    from aiida.cmdline.utils.common import get_process_function_report
 
     code = aiida_code_installed(default_calc_job_plugin='core.arithmetic.add', filepath_executable='/bin/bash')
 
@@ -306,7 +302,11 @@ def make_a_builder(sleep_seconds=0):
         # kill should restart EBM and be not successful in EBM
         # this tests if the old task is cancelled and restarted successfully
         run_cli_command(cmd_process.process_kill, [str(node.pk), '--wait'])
-        await_condition(lambda: not node.is_killed, timeout=kill_timeout)
+        await_condition(
+            lambda: 'Found active scheduler job cancelation that will be rescheduled.'
+            in get_process_function_report(node),
+            timeout=kill_timeout,
+        )
 
         # force kill should skip EBM and successfully kill the process
         run_cli_command(cmd_process.process_kill, [str(node.pk), '-F', '--wait'])
@@ -826,8 +826,6 @@ def test_process_kill(submit_and_await, run_cli_command, aiida_code_installed):
     assert result.exit_code == ExitCode.USAGE_ERROR
     assert len(result.output_lines) > 0
 
-    from aiida.orm import Int
-
     code = aiida_code_installed(default_calc_job_plugin='core.arithmetic.add', filepath_executable='/bin/bash')
     builder = code.get_builder()
     builder.x = Int(2)