Skip to content

Commit e4d0eb5

Browse files
nthmost-orkesclaude
andcommitted
fix: auto-start workers in context manager and warn on RUNNING+failed tasks
- TaskHandler.__enter__ now calls start_processes() so `with TaskHandler(...) as h:` works out of the box without a separate h.start_processes() call. start_processes() is idempotent via _processes_started guard, so existing code that calls it explicitly inside the with-block is unaffected. Fixes conductor-oss/getting-started#42. - WorkflowExecutor.execute() and execute_workflow() now log a WARNING when the workflow returns RUNNING after the wait timeout but a task is already in FAILED/FAILED_WITH_TERMINAL_ERROR state, surfacing the failure reason that would otherwise be invisible to the caller. Fixes conductor-oss/getting-started#41. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e5ff5cd commit e4d0eb5

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

src/conductor/client/automator/task_handler.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,11 @@ def __init__(
325325
self._next_restart_at: List[float] = [0.0 for _ in self.workers]
326326
# Lock to protect process list during concurrent access (monitor thread vs main thread)
327327
self._process_lock = threading.Lock()
328+
self._processes_started = False
328329
logger.info("TaskHandler initialized")
329330

330331
def __enter__(self):
332+
self.start_processes()
331333
return self
332334

333335
def __exit__(self, exc_type, exc_value, traceback):
@@ -341,11 +343,15 @@ def stop_processes(self) -> None:
341343
with self._process_lock:
342344
self.__stop_task_runner_processes()
343345
self.__stop_metrics_provider_process()
346+
self._processes_started = False
344347
logger.info("Stopped worker processes...")
345348
self.queue.put(None)
346349
self.logger_process.terminate()
347350

348351
def start_processes(self) -> None:
352+
if self._processes_started:
353+
return
354+
self._processes_started = True
349355
logger.info("Starting worker processes...")
350356
freeze_support()
351357
self._monitor_stop_event.clear()

src/conductor/client/workflow/executor/workflow_executor.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
from __future__ import annotations
2+
import logging
23
import uuid
34
from typing import Any, Dict, List, Optional, TYPE_CHECKING
45

6+
logger = logging.getLogger(__name__)
7+
58
from typing_extensions import Self
69

710
from conductor.client.configuration.configuration import Configuration
@@ -122,12 +125,26 @@ def execute(self, name: str, version: Optional[int] = None, workflow_input: Any
122125
if domain is not None:
123126
request.task_to_domain = {"*": domain}
124127

125-
return self.workflow_client.execute_workflow(
128+
workflow_run = self.workflow_client.execute_workflow(
126129
start_workflow_request=request,
127130
request_id=request_id,
128131
wait_until_task_ref=wait_until_task_ref,
129132
wait_for_seconds=wait_for_seconds,
130133
)
134+
if getattr(workflow_run, 'status', None) == 'RUNNING':
135+
try:
136+
full = self.workflow_client.get_workflow(workflow_run.workflow_id, include_tasks=True)
137+
failed = [t for t in (full.tasks or []) if t.status in ('FAILED', 'FAILED_WITH_TERMINAL_ERROR')]
138+
if failed:
139+
t = failed[0]
140+
reason = getattr(t, 'reason_for_incompletion', None) or 'see server logs'
141+
logger.warning(
142+
"Workflow %s returned RUNNING after timeout, but task '%s' has status %s: %s",
143+
workflow_run.workflow_id, t.reference_task_name, t.status, reason,
144+
)
145+
except Exception:
146+
pass
147+
return workflow_run
131148

132149
def remove_workflow(self, workflow_id: str, archive_workflow: Optional[bool] = None) -> None:
133150
"""Removes the workflow permanently from the system"""

tests/unit/automator/test_task_handler_coverage.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,12 +663,19 @@ def test_context_manager_enter(self, mock_process_class, mock_import, mock_loggi
663663

664664
@patch('conductor.client.automator.task_handler._setup_logging_queue')
665665
@patch('importlib.import_module')
666-
def test_context_manager_exit(self, mock_import, mock_logging):
666+
@patch('conductor.client.automator.task_handler.Process')
667+
def test_context_manager_exit(self, mock_process_class, mock_import, mock_logging):
667668
"""Test context manager __exit__ method."""
668669
mock_queue = Mock()
669670
mock_logger_process = Mock()
670671
mock_logging.return_value = (mock_logger_process, mock_queue)
671672

673+
mock_process = Mock()
674+
mock_process.terminate = Mock()
675+
mock_process.kill = Mock()
676+
mock_process.is_alive = Mock(return_value=False)
677+
mock_process_class.return_value = mock_process
678+
672679
worker = ClassWorker('test_task')
673680
handler = TaskHandler(
674681
workers=[worker],
@@ -679,10 +686,16 @@ def test_context_manager_exit(self, mock_import, mock_logging):
679686
# Override the queue and logger_process with fresh mocks
680687
handler.queue = Mock()
681688
handler.logger_process = Mock()
689+
handler.logger_process.is_alive = Mock(return_value=False)
690+
handler.metrics_provider_process = Mock()
691+
handler.metrics_provider_process.terminate = Mock()
692+
handler.metrics_provider_process.is_alive = Mock(return_value=False)
682693

683694
# Mock terminate on all processes
684695
for process in handler.task_runner_processes:
685696
process.terminate = Mock()
697+
process.kill = Mock()
698+
process.is_alive = Mock(return_value=False)
686699

687700
with handler:
688701
pass

0 commit comments

Comments
 (0)