Skip to content

Commit 3e9a8aa

Browse files
fix: resolve reviewer issues - timeout blocking, BaseTool import, boolean conversion
- Fix ThreadPoolExecutor timeout blocking by using non-blocking shutdown - Replace BaseTool import error with safe tool instantiation pattern - Fix case-sensitive boolean conversion for handoff_detect_cycles - Consolidate duplicate tool loading code in ToolResolver - Improve per-file error isolation in directory tool loading Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent 7cadf19 commit 3e9a8aa

3 files changed

Lines changed: 26 additions & 48 deletions

File tree

src/praisonai/praisonai/agents_generator.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ def _merge_cli_config(self, config, cli_config):
358358
if 'handoff_max_concurrent' in cli_config:
359359
handoff_config['max_concurrent'] = cli_config['handoff_max_concurrent']
360360
if 'handoff_detect_cycles' in cli_config:
361-
handoff_config['detect_cycles'] = cli_config['handoff_detect_cycles'] == 'true'
361+
handoff_config['detect_cycles'] = cli_config['handoff_detect_cycles'].lower() == 'true'
362362

363363
if handoff_config:
364364
agent_overrides['handoff'] = handoff_config
@@ -608,12 +608,17 @@ def generate_crew_and_kickoff(self):
608608
except Exception as e:
609609
self.logger.warning(f"Error collecting YAML tool references: {e}")
610610

611-
# Add tools from class names
611+
# Add tools from class names - use tool_resolver to check tool validity
612612
for tool_class in self.tools:
613-
if isinstance(tool_class, type) and BaseTool and issubclass(tool_class, BaseTool):
614-
tool_name = tool_class.__name__
615-
tools_dict[tool_name] = tool_class()
616-
self.logger.debug(f"Added tool: {tool_name}")
613+
if isinstance(tool_class, type):
614+
try:
615+
# Try to instantiate the tool to validate it
616+
tool_instance = tool_class()
617+
tool_name = tool_class.__name__
618+
tools_dict[tool_name] = tool_instance
619+
self.logger.debug(f"Added tool: {tool_name}")
620+
except Exception as e:
621+
self.logger.warning(f"Failed to instantiate tool class {tool_class.__name__}: {e}")
617622

618623
root_directory = os.getcwd()
619624
tools_py_path = os.path.join(root_directory, 'tools.py')

src/praisonai/praisonai/scheduler/agent_scheduler.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,14 +247,14 @@ def _execute_with_retry(self, max_retries: int):
247247
# Execute with timeout if specified
248248
if self.timeout:
249249
executor = ThreadPoolExecutor(max_workers=1)
250+
future = executor.submit(self._executor.execute, self.task)
250251
try:
251-
future = executor.submit(self._executor.execute, self.task)
252-
try:
253-
result = future.result(timeout=self.timeout)
254-
except FuturesTimeout:
255-
future.cancel()
256-
raise TimeoutError(f"Execution exceeded {self.timeout}s timeout")
257-
finally:
252+
result = future.result(timeout=self.timeout)
253+
except FuturesTimeout as e:
254+
future.cancel()
255+
executor.shutdown(wait=False, cancel_futures=True)
256+
raise TimeoutError(f"Execution exceeded {self.timeout}s timeout") from e
257+
else:
258258
executor.shutdown(wait=False, cancel_futures=True)
259259
else:
260260
result = self._executor.execute(self.task)

src/praisonai/praisonai/tool_resolver.py

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -454,34 +454,7 @@ def get_local_tool_classes(self) -> Dict[str, Any]:
454454
module = load_user_module(self._tools_py_path, name="tools_module")
455455
if module is None:
456456
return {}
457-
458-
# Import the necessary classes (matching agents_generator.py logic)
459-
BaseTool = None
460-
PRAISONAI_TOOLS_AVAILABLE = False
461-
try:
462-
from praisonai_tools import BaseTool
463-
PRAISONAI_TOOLS_AVAILABLE = True
464-
except ImportError:
465-
try:
466-
from praisonai.tools import BaseTool
467-
PRAISONAI_TOOLS_AVAILABLE = True
468-
except ImportError:
469-
pass
470-
471-
result = {}
472-
for name, obj in inspect.getmembers(module,
473-
lambda x: inspect.isclass(x) and (
474-
x.__module__.startswith('langchain_community.tools') or
475-
(PRAISONAI_TOOLS_AVAILABLE and BaseTool and issubclass(x, BaseTool))
476-
) and x is not BaseTool):
477-
try:
478-
result[name] = obj()
479-
logger.debug(f"Loaded local tool class: {name}")
480-
except Exception as e:
481-
logger.warning(f"Error instantiating tool class {name}: {e}")
482-
continue
483-
484-
return result
457+
return self._extract_tool_classes(module)
485458
except Exception as e:
486459
logger.warning(f"Error loading tool classes from {self._tools_py_path}: {e}")
487460
return {}
@@ -499,15 +472,15 @@ def get_local_tool_classes_from_dir(self, tools_dir: "os.PathLike|str") -> Dict[
499472
from ._safe_loader import load_user_module
500473

501474
classes: Dict[str, Any] = {}
502-
try:
503-
for py_file in Path(tools_dir).glob("*.py"):
504-
if py_file.name.startswith("__"):
505-
continue
475+
for py_file in Path(tools_dir).glob("*.py"):
476+
if py_file.name.startswith("__"):
477+
continue
478+
try:
506479
module = load_user_module(py_file, name=f"tools_{py_file.stem}")
507480
if module is not None:
508-
classes.update(self._extract_tool_classes(module)) # reuse resolver's own predicate
509-
except Exception as e:
510-
logger.warning(f"Error loading tool classes from directory {tools_dir}: {e}")
481+
classes.update(self._extract_tool_classes(module))
482+
except Exception as e:
483+
logger.warning(f"Error loading tool classes from file {py_file}: {e}")
511484
return classes
512485

513486
def _extract_tool_classes(self, module):

0 commit comments

Comments
 (0)