Skip to content

Commit 3aeb49a

Browse files
GuillaumeDSMGuillaumeDSM
committed
CI fix attempt
1 parent 56dc80c commit 3aeb49a

6 files changed

Lines changed: 137 additions & 31 deletions

File tree

‎packages/commons/octobot_commons/process_util.py‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,30 @@ def spawn_managed_subprocess(
7070
)
7171

7272

73-
def pid_is_running(pid: int) -> bool:
74-
"""Best-effort: whether ``pid`` denotes a running OS process."""
73+
def pid_is_running(pid: int) -> bool: # pylint: disable=too-many-return-statements
74+
"""Best-effort: whether ``pid`` denotes a running OS process (zombies are treated as not running)."""
7575
if pid <= 0:
7676
return False
77-
if psutil is not None:
78-
try:
79-
return psutil.Process(pid).is_running()
80-
except psutil.NoSuchProcess:
77+
try:
78+
proc = psutil.Process(pid)
79+
except psutil.NoSuchProcess:
80+
return False
81+
except psutil.AccessDenied:
82+
return True
83+
try:
84+
if proc.status() == psutil.STATUS_ZOMBIE:
8185
return False
82-
except psutil.AccessDenied:
83-
return True
84-
return pid > 0
86+
except psutil.ZombieProcess:
87+
return False
88+
except psutil.NoSuchProcess:
89+
# PID can disappear between Process() creation and status() (e.g. SIGTERM on Windows).
90+
return False
91+
try:
92+
return proc.is_running()
93+
except psutil.ZombieProcess:
94+
return False
95+
except psutil.NoSuchProcess:
96+
return False
8597

8698

8799
def request_graceful_stop_via_sigterm(

‎packages/commons/tests/test_process_util.py‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,23 +81,47 @@ def test_non_positive_pid_is_false(self):
8181

8282
def test_with_psutil_process_running_true(self):
8383
fake_process = mock.Mock()
84+
fake_process.status.return_value = process_util.psutil.STATUS_RUNNING
8485
fake_process.is_running.return_value = True
8586
with mock.patch.object(process_util.psutil, "Process", return_value=fake_process):
8687
assert process_util.pid_is_running(42) is True
8788
fake_process.is_running.assert_called_once()
8889

90+
def test_with_psutil_zombie_is_not_running(self):
91+
"""``is_running()`` can stay True for Linux zombies; we treat them as stopped for lifecycle waits."""
92+
fake_process = mock.Mock()
93+
fake_process.status.return_value = process_util.psutil.STATUS_ZOMBIE
94+
fake_process.is_running.return_value = True
95+
with mock.patch.object(process_util.psutil, "Process", return_value=fake_process):
96+
assert process_util.pid_is_running(42) is False
97+
fake_process.is_running.assert_not_called()
98+
99+
def test_with_psutil_zombie_process_exception_from_status(self):
100+
fake_process = mock.Mock()
101+
fake_process.status.side_effect = process_util.psutil.ZombieProcess(42)
102+
with mock.patch.object(process_util.psutil, "Process", return_value=fake_process):
103+
assert process_util.pid_is_running(42) is False
104+
89105
def test_with_psutil_no_such_process(self):
90106
fake_process_constructor = mock.Mock(
91107
side_effect=process_util.psutil.NoSuchProcess(42),
92108
)
93109
with mock.patch.object(process_util.psutil, "Process", fake_process_constructor):
94110
assert process_util.pid_is_running(42) is False
95111

96-
def test_without_psutil_true_for_positive_pid(self):
97-
"""Fallback path when ``psutil`` is absent: naive ``pid > 0`` (see ``process_util``)."""
98-
with mock.patch.object(process_util, "psutil", None):
99-
assert process_util.pid_is_running(1) is True
100-
assert process_util.pid_is_running(999_999_999) is True
112+
def test_with_psutil_no_such_process_from_status_after_process_ctor(self):
113+
"""Process() succeeds but status() raises (race: exited between ctor and probe, common on Windows)."""
114+
fake_process = mock.Mock()
115+
fake_process.status.side_effect = process_util.psutil.NoSuchProcess(42)
116+
with mock.patch.object(process_util.psutil, "Process", return_value=fake_process):
117+
assert process_util.pid_is_running(42) is False
118+
119+
def test_with_psutil_no_such_process_from_is_running(self):
120+
fake_process = mock.Mock()
121+
fake_process.status.return_value = process_util.psutil.STATUS_RUNNING
122+
fake_process.is_running.side_effect = process_util.psutil.NoSuchProcess(42)
123+
with mock.patch.object(process_util.psutil, "Process", return_value=fake_process):
124+
assert process_util.pid_is_running(42) is False
101125

102126

103127
class TestRequestGracefulStopViaSigterm:

‎packages/flow/octobot_flow/logic/actions/actions_executor.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,14 @@ async def _execute_post_iteration_actions(
163163
dsl_executor,
164164
octobot_commons.dsl_interpreter.OperatorSignal.UPDATE_CONFIG.value,
165165
)
166-
# special case: the dag pending action got executed: return reset details to reset the DAG action
167166
if executed_dag_action is None or dag_action_result is None:
168-
self._get_logger().error(f"Unexpected no return form config update")
167+
raise octobot_flow.errors.AutomationActionError(
168+
"update_automation_configuration did not receive a result from the signaled DAG action."
169+
)
170+
if not dag_action_result.succeeded():
171+
raise octobot_flow.errors.AutomationActionError(
172+
f"update_automation_configuration failed: {dag_action_result.error}"
173+
)
169174
return self._create_recall_dag_details_if_necessary(
170175
executed_dag_action.id,
171176
dag_action_result.result,

‎packages/flow/octobot_flow/logic/dsl/dsl_action_execution_context.py‎

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,17 @@
99
import octobot_flow.enums
1010

1111

12+
def _dsl_action_error_call_result(
13+
action: octobot_flow.entities.DSLScriptActionDetails,
14+
error_status,
15+
) -> octobot_commons.dsl_interpreter.DSLCallResult:
16+
action.complete(error_status=error_status)
17+
return octobot_commons.dsl_interpreter.DSLCallResult(
18+
statement=action.get_resolved_dsl_script(),
19+
error=error_status,
20+
)
21+
22+
1223
def dsl_action_execution(func):
1324
async def _action_execution_error_handler_wrapper(
1425
self, action: octobot_flow.entities.DSLScriptActionDetails, **kwargs
@@ -26,31 +37,54 @@ async def _action_execution_error_handler_wrapper(
2637
else:
2738
action.complete(error_status=call_result.error)
2839
return call_result
29-
except octobot_trading.errors.DisabledFundsTransferError as err:
30-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.DISABLED_FUNDS_TRANSFER_ERROR.value)
40+
except octobot_trading.errors.DisabledFundsTransferError:
41+
return _dsl_action_error_call_result(
42+
action,
43+
octobot_flow.enums.ActionErrorStatus.DISABLED_FUNDS_TRANSFER_ERROR.value,
44+
)
3145
except octobot_trading.errors.MissingMinimalExchangeTradeVolume as err:
3246
octobot_commons.logging.get_logger("action_execution").exception(err, True, f"Missing minimal exchange trade volume error: {err}")
33-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.INVALID_ORDER.value)
34-
except (octobot_trading.errors.UnsupportedHedgeContractError, octobot_trading.errors.InvalidPositionSide) as err:
35-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.UNSUPPORTED_HEDGE_POSITION.value)
36-
except octobot_trading.errors.ExchangeAccountSymbolPermissionError as err:
37-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.SYMBOL_INCOMPATIBLE_WITH_ACCOUNT.value)
38-
except octobot_commons.errors.InvalidParameterFormatError as err:
39-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.INVALID_SIGNAL_FORMAT.value)
47+
return _dsl_action_error_call_result(
48+
action,
49+
octobot_flow.enums.ActionErrorStatus.INVALID_ORDER.value,
50+
)
51+
except (octobot_trading.errors.UnsupportedHedgeContractError, octobot_trading.errors.InvalidPositionSide):
52+
return _dsl_action_error_call_result(
53+
action,
54+
octobot_flow.enums.ActionErrorStatus.UNSUPPORTED_HEDGE_POSITION.value,
55+
)
56+
except octobot_trading.errors.ExchangeAccountSymbolPermissionError:
57+
return _dsl_action_error_call_result(
58+
action,
59+
octobot_flow.enums.ActionErrorStatus.SYMBOL_INCOMPATIBLE_WITH_ACCOUNT.value,
60+
)
61+
except octobot_commons.errors.InvalidParameterFormatError:
62+
return _dsl_action_error_call_result(
63+
action,
64+
octobot_flow.enums.ActionErrorStatus.INVALID_SIGNAL_FORMAT.value,
65+
)
4066
except octobot_trading.errors.NotSupportedOrderTypeError as err:
41-
if err.order_type == octobot_trading.enums.TraderOrderType.STOP_LOSS:
42-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.UNSUPPORTED_STOP_ORDER.value)
43-
else:
44-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.INVALID_ORDER.value)
67+
error_status_value = (
68+
octobot_flow.enums.ActionErrorStatus.UNSUPPORTED_STOP_ORDER.value
69+
if err.order_type == octobot_trading.enums.TraderOrderType.STOP_LOSS
70+
else octobot_flow.enums.ActionErrorStatus.INVALID_ORDER.value
71+
)
72+
return _dsl_action_error_call_result(action, error_status_value)
4573
except octobot_trading.errors.BlockchainWalletError as err:
4674
octobot_commons.logging.get_logger("action_execution").exception(err, True, f"Blockchain wallet error: {err}")
47-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.BLOCKCHAIN_WALLET_ERROR.value)
75+
return _dsl_action_error_call_result(
76+
action,
77+
octobot_flow.enums.ActionErrorStatus.BLOCKCHAIN_WALLET_ERROR.value,
78+
)
4879
except Exception as err:
4980
octobot_commons.logging.get_logger("action_execution").exception(
5081
err,
5182
True,
5283
f"Failed to interpret DSL script '{action.get_summary(not octobot_commons.constants.ALLOW_PRIVATE_DATA_LOGS)}' "
5384
f"for action: {action.id}: {err}"
5485
)
55-
action.complete(error_status=octobot_flow.enums.ActionErrorStatus.INTERNAL_ERROR.value)
86+
return _dsl_action_error_call_result(
87+
action,
88+
octobot_flow.enums.ActionErrorStatus.INTERNAL_ERROR.value,
89+
)
5690
return _action_execution_error_handler_wrapper

‎packages/tentacles/Meta/DSL_operators/octobot_process_operators/octobot_process_ops.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,9 @@ async def pre_compute(self) -> None:
769769
raise commons_errors.DSLInterpreterError(
770770
"run_octobot_process(execution_stop) requires last_execution_result from a prior run_octobot_process call.",
771771
)
772+
if not self.is_process_running():
773+
self.value = {"status": "already_stopped", "reason": "not_running"}
774+
return
772775
self.value = self.request_graceful_stop(logger=_get_logger())
773776
return
774777
working_directory = os.path.normpath(os.getcwd())

‎packages/tentacles/Meta/DSL_operators/octobot_process_operators/tests/test_octobot_process_ops.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,34 @@ async def test_execution_stop_dead_child_is_already_stopped(self):
10941094
assert isinstance(op.value, dict)
10951095
assert op.value["status"] == "already_stopped"
10961096

1097+
async def test_execution_stop_short_circuits_without_sigterm_when_not_running(self):
1098+
"""STOP branch returns already_stopped before ``request_graceful_stop`` when ``is_process_running`` is false."""
1099+
inner = _stop_test_ensure_state_dict("http://127.0.0.1:7")
1100+
op = octobot_process_ops.EnsureOctobotProcessOperator(
1101+
user_folder="u1",
1102+
profile_data=_MINIMAL_PROFILE_DATA,
1103+
last_execution_result=_re_calling_ensure_value(inner),
1104+
)
1105+
graceful_stop_mock = mock.Mock()
1106+
with (
1107+
mock.patch.object(
1108+
dsl_interpreter.ProcessBoundOperatorMixin,
1109+
"is_process_running",
1110+
return_value=False,
1111+
),
1112+
mock.patch.object(
1113+
octobot_process_ops.EnsureOctobotProcessOperator,
1114+
"request_graceful_stop",
1115+
new=graceful_stop_mock,
1116+
),
1117+
octobot_process_ops.EnsureOctobotProcessOperator.set_execution_signal(
1118+
dsl_interpreter.OperatorSignal.STOP.value
1119+
),
1120+
):
1121+
await op.pre_compute()
1122+
graceful_stop_mock.assert_not_called()
1123+
assert op.value == {"status": "already_stopped", "reason": "not_running"}
1124+
10971125
async def test_execution_stop_os_kill_failure_raises(self):
10981126
inner = _stop_test_ensure_state_dict("http://127.0.0.1:7")
10991127
op = octobot_process_ops.EnsureOctobotProcessOperator(

0 commit comments

Comments
 (0)