mirror of
https://github.com/MervinPraison/PraisonAI.git
synced 2026-11-07 12:47:55 -10:00
Merge pull request #3634 from MervinPraison/claude/issue-3633-20260804-0833
fix: surface FileNotFound, unify auto-generator defaults, harden push polling fallback
This commit is contained in:
5 files changed
+151
-41
No files matched your search
@@ -1109,12 +1109,12 @@ class AgentsGenerator:
|
||||
else:
|
||||
if self.agent_file in ('/app/api:app', 'api:app'):
|
||||
self.agent_file = 'agents.yaml'
|
||||
try:
|
||||
with open(self.agent_file, 'r') as f:
|
||||
config = _yaml_safe_load(f)
|
||||
except FileNotFoundError:
|
||||
_rich_print(f"File not found: {self.agent_file}")
|
||||
return None
|
||||
# Let FileNotFoundError surface so the Python API, HTTP handlers,
|
||||
# and tests can distinguish "no such file" from "no result".
|
||||
# Downgrading it to a print (returning None) made every public
|
||||
# entry point return a silent None for a missing file.
|
||||
with open(self.agent_file, 'r') as f:
|
||||
config = _yaml_safe_load(f)
|
||||
|
||||
config = _normalize_yaml_config(config or {})
|
||||
|
||||
@@ -1153,8 +1153,6 @@ class AgentsGenerator:
|
||||
This function first loads the agent configuration from the specified file. It then initializes the tools required for the agents based on the specified framework. If the specified framework is "autogen", it loads the LLM configuration dynamically and creates an AssistantAgent for each role in the configuration. It then adds tools to the agents if specified in the configuration. Finally, it prepares tasks for the agents based on the configuration and initiates the tasks using the crew of agents. If the specified framework is not "autogen", it creates a crew of agents and initiates tasks based on the configuration.
|
||||
"""
|
||||
config = self._load_config()
|
||||
if config is None:
|
||||
return
|
||||
from .observability.hooks import observability_session
|
||||
if self._is_workflow_yaml(config):
|
||||
# Bracket the workflow run in the same observability session the
|
||||
@@ -1201,8 +1199,6 @@ class AgentsGenerator:
|
||||
Generates a crew of agents and initiates tasks based on the provided configuration.
|
||||
"""
|
||||
config = await self._aload_config()
|
||||
if config is None:
|
||||
return
|
||||
import asyncio
|
||||
from .observability.hooks import observability_session
|
||||
if self._is_workflow_yaml(config):
|
||||
|
||||
@@ -370,6 +370,52 @@ class BaseAutoGenerator:
|
||||
self._core_client = None # lazy, per-instance core OpenAIClient
|
||||
self._client_lock = threading.Lock()
|
||||
|
||||
def _resolve_framework(self, requested: Optional[str], registry,
|
||||
require_workflow: bool = False):
|
||||
"""Resolve and validate a framework via the shared adapter registry.
|
||||
|
||||
Single source of truth for default selection: when ``requested`` is
|
||||
falsy (``None`` or empty string), defer to ``registry.pick_default()``
|
||||
— the same selector ``praisonai.run(...)`` uses — instead of hardcoding
|
||||
a per-class default. Validates the adapter exists and is installed so
|
||||
both auto-generators fail loudly and consistently.
|
||||
|
||||
When ``require_workflow`` is set, the resolved adapter must also
|
||||
advertise ``SUPPORTS_WORKFLOW``. This keeps ``WorkflowAutoGenerator``
|
||||
from silently picking an installed-but-non-workflow default (e.g. when
|
||||
the native ``praisonai`` adapter is absent) and then emitting a
|
||||
workflow YAML that ``AgentsGenerator`` would reject at run time.
|
||||
"""
|
||||
if not requested:
|
||||
requested = registry.pick_default()
|
||||
try:
|
||||
adapter = registry.create(requested)
|
||||
except ValueError as e:
|
||||
raise ImportError(
|
||||
f"Unknown framework '{requested}'. Available frameworks: "
|
||||
f"{', '.join(registry.list_registered())}"
|
||||
) from e
|
||||
if not adapter.is_available():
|
||||
install_hint = getattr(adapter, "install_hint", f"pip install {requested}")
|
||||
raise ImportError(
|
||||
f"{adapter.name} is not installed. Please install with:\n {install_hint}"
|
||||
)
|
||||
if require_workflow and getattr(adapter, "SUPPORTS_WORKFLOW", False) is not True:
|
||||
from .framework_adapters.registry import adapter_capability
|
||||
supported = [
|
||||
name for name in registry.list_registered()
|
||||
if adapter_capability(name, "SUPPORTS_WORKFLOW", registry=registry) is True
|
||||
]
|
||||
hint = (
|
||||
f" Frameworks supporting workflow execution: "
|
||||
f"{', '.join(sorted(set(supported)))}." if supported else ""
|
||||
)
|
||||
raise ImportError(
|
||||
f"Framework '{adapter.name}' does not support workflow "
|
||||
f"generation (SUPPORTS_WORKFLOW is not set).{hint}"
|
||||
)
|
||||
return adapter
|
||||
|
||||
def _get_core_client(self):
|
||||
"""Get or create the core-owned ``OpenAIClient`` for this instance.
|
||||
|
||||
@@ -891,7 +937,7 @@ class AutoGenerator(BaseAutoGenerator):
|
||||
"""
|
||||
|
||||
def __init__(self, topic="Movie Story writing about AI", agent_file="test.yaml",
|
||||
framework="crewai", config_list: Optional[List[Dict]] = None,
|
||||
framework: Optional[str] = None, config_list: Optional[List[Dict]] = None,
|
||||
pattern: str = "sequential", single_agent: bool = False,
|
||||
adapter_registry=None):
|
||||
"""
|
||||
@@ -900,7 +946,10 @@ class AutoGenerator(BaseAutoGenerator):
|
||||
Args:
|
||||
topic: The task/topic for agent generation
|
||||
agent_file: Output YAML file name
|
||||
framework: Framework to use (crewai, autogen, praisonai)
|
||||
framework: Framework to use (praisonai, crewai, autogen). When
|
||||
omitted, resolves via the shared registry default selector —
|
||||
the same one ``praisonai.run(...)`` uses — so the
|
||||
generate → run round-trip agrees on defaults.
|
||||
config_list: Optional LLM configuration
|
||||
pattern: Workflow pattern (sequential, parallel, routing, orchestrator-workers, evaluator-optimizer)
|
||||
single_agent: If True, generate a single agent instead of a team
|
||||
@@ -910,28 +959,16 @@ class AutoGenerator(BaseAutoGenerator):
|
||||
# Initialize base class first (handles config_list and client)
|
||||
super().__init__(config_list=config_list)
|
||||
|
||||
# Validate framework availability using adapter registry
|
||||
# Resolve + validate framework using the shared adapter registry.
|
||||
from .framework_adapters.registry import get_default_registry
|
||||
|
||||
self._adapter_registry = adapter_registry or get_default_registry()
|
||||
try:
|
||||
adapter = self._adapter_registry.create(framework)
|
||||
except ValueError as e:
|
||||
raise ImportError(
|
||||
f"Unknown framework '{framework}'. Available frameworks: "
|
||||
f"{', '.join(self._adapter_registry.list_registered())}"
|
||||
) from e
|
||||
adapter = self._resolve_framework(framework, self._adapter_registry)
|
||||
|
||||
# Use safe fallbacks for new adapter attributes
|
||||
install_hint = getattr(adapter, "install_hint", f"pip install {framework}")
|
||||
requires_tools_extra = bool(getattr(adapter, "requires_tools_extra", False))
|
||||
|
||||
if not adapter.is_available():
|
||||
raise ImportError(f"{adapter.name} is not installed. Please install with:\n {install_hint}")
|
||||
|
||||
# Check tools availability if required by this framework
|
||||
if requires_tools_extra and not is_available("praisonai_tools"):
|
||||
logger.warning(f"Tools are not available for {framework}. To use tools, install:\n {install_hint}")
|
||||
install_hint = getattr(adapter, "install_hint", f"pip install {adapter.name}")
|
||||
if bool(getattr(adapter, "requires_tools_extra", False)) and not is_available("praisonai_tools"):
|
||||
logger.warning(f"Tools are not available for {adapter.name}. To use tools, install:\n {install_hint}")
|
||||
|
||||
# Retain the resolved adapter as the canonical dispatch object (mirrors
|
||||
# AgentsGenerator, which stores self.framework_adapter). Downstream code
|
||||
@@ -939,8 +976,8 @@ class AutoGenerator(BaseAutoGenerator):
|
||||
self._adapter = adapter
|
||||
self.topic = topic
|
||||
self.agent_file = agent_file
|
||||
# Authoritative framework name comes from the adapter, not the raw arg.
|
||||
self.framework = adapter.name or framework or "praisonai"
|
||||
# Authoritative framework name comes from the resolved adapter.
|
||||
self.framework = adapter.name
|
||||
self.pattern = pattern
|
||||
self.single_agent = single_agent
|
||||
|
||||
@@ -1380,8 +1417,9 @@ class WorkflowAutoGenerator(BaseAutoGenerator):
|
||||
def __init__(self, topic: str = "Research and write about AI",
|
||||
workflow_file: str = "workflow.yaml",
|
||||
config_list: Optional[List[Dict]] = None,
|
||||
framework: str = "praisonai",
|
||||
single_agent: bool = False):
|
||||
framework: Optional[str] = None,
|
||||
single_agent: bool = False,
|
||||
adapter_registry=None):
|
||||
"""
|
||||
Initialize the WorkflowAutoGenerator.
|
||||
|
||||
@@ -1389,15 +1427,28 @@ class WorkflowAutoGenerator(BaseAutoGenerator):
|
||||
topic: The task/topic for the workflow
|
||||
workflow_file: Output file name
|
||||
config_list: Optional LLM configuration
|
||||
framework: Framework to use (praisonai, crewai, autogen)
|
||||
framework: Framework to use (praisonai, crewai, autogen). When
|
||||
omitted, resolves via the shared registry default selector so
|
||||
the generate → run round-trip agrees on defaults. The adapter
|
||||
is validated here rather than deferring the failure to a later
|
||||
``praisonai run`` invocation.
|
||||
single_agent: If True, generate a single agent workflow
|
||||
"""
|
||||
# Initialize base class (handles config_list and client)
|
||||
super().__init__(config_list=config_list)
|
||||
|
||||
# Resolve + validate framework through the same single source of truth
|
||||
# AutoGenerator and praisonai.run() use, instead of hardcoding a
|
||||
# different per-class default and skipping validation.
|
||||
from .framework_adapters.registry import get_default_registry
|
||||
self._adapter_registry = adapter_registry or get_default_registry()
|
||||
adapter = self._resolve_framework(
|
||||
framework, self._adapter_registry, require_workflow=True
|
||||
)
|
||||
self._adapter = adapter
|
||||
self.topic = topic
|
||||
self.workflow_file = workflow_file
|
||||
self.framework = framework
|
||||
self.framework = adapter.name
|
||||
self.single_agent = single_agent
|
||||
|
||||
def recommend_pattern_llm(self, topic: Optional[str] = None) -> Any:
|
||||
|
||||
@@ -129,6 +129,23 @@ class PushClient:
|
||||
def is_connected(self) -> bool:
|
||||
return self._connected and self._transport is not None and self._transport.is_connected
|
||||
|
||||
@property
|
||||
def supports_publish(self) -> bool:
|
||||
"""Whether producer operations are available on the active transport.
|
||||
|
||||
``False`` after a WebSocket → polling fallback: the polling contract
|
||||
has no publish/create/presence-query routes, so ``publish``,
|
||||
``create_channel`` and ``get_presence`` would raise. Callers can gate
|
||||
their producer code on this instead of discovering the drop at call time.
|
||||
|
||||
Requires an active connection: before connect, after ``disconnect()``,
|
||||
or after a failed connect that never entered polling mode there is no
|
||||
transport to publish through, so this reports ``False`` rather than a
|
||||
false positive that would let gated producer calls proceed and fail
|
||||
with ``ConnectionError``.
|
||||
"""
|
||||
return self.is_connected and not self._using_polling
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Channel operations
|
||||
# ------------------------------------------------------------------
|
||||
@@ -383,7 +400,12 @@ class PushClient:
|
||||
await self._transport.connect()
|
||||
self._connected = True
|
||||
self._using_polling = True
|
||||
logger.info("Switched to polling transport")
|
||||
logger.warning(
|
||||
"PushClient fell back to polling transport. Unavailable "
|
||||
"operations under polling: publish, create_channel, "
|
||||
"get_presence (they will raise NotImplementedError). Gate "
|
||||
"producer code on the `supports_publish` property."
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error("Polling transport failed: %s", e)
|
||||
self._connected = False
|
||||
@@ -157,6 +157,18 @@ class PollingTransport:
|
||||
headers=headers,
|
||||
) as resp:
|
||||
resp.raise_for_status()
|
||||
else:
|
||||
# The polling contract does not currently include publish, create,
|
||||
# or presence-query endpoints, so silently returning here would
|
||||
# drop the message. Surface the drop so callers can decide (fail
|
||||
# startup, retry over WS, or disable fallback) instead of losing it.
|
||||
raise NotImplementedError(
|
||||
f"PollingTransport does not support message type {msg_type!r}. "
|
||||
"This operation requires the WebSocket transport. Either disable "
|
||||
"fallback_to_polling, or add server-side /api/push/poll/publish + "
|
||||
"/api/push/poll/create + /api/push/poll/presence routes and map "
|
||||
"them here."
|
||||
)
|
||||
|
||||
async def receive(self) -> Dict[str, Any]:
|
||||
"""Long-poll for the next message."""
|
||||
|
||||
@@ -822,27 +822,56 @@ class TestWorkflowMergeSupport:
|
||||
class TestWorkflowFrameworkSupport:
|
||||
"""Test suite for framework support in WorkflowAutoGenerator."""
|
||||
|
||||
@staticmethod
|
||||
def _fake_registry(name="crewai", default="praisonai", supports_workflow=True):
|
||||
"""Registry double whose adapter reports available, so framework
|
||||
validation passes without the framework actually being installed.
|
||||
|
||||
``supports_workflow`` mirrors the adapter ``SUPPORTS_WORKFLOW`` flag the
|
||||
WorkflowAutoGenerator now enforces via ``require_workflow=True``.
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
adapter = MagicMock()
|
||||
adapter.name = name
|
||||
adapter.is_available.return_value = True
|
||||
adapter.SUPPORTS_WORKFLOW = supports_workflow
|
||||
registry = MagicMock()
|
||||
registry.create.return_value = adapter
|
||||
registry.pick_default.return_value = default
|
||||
return registry
|
||||
|
||||
def test_workflow_generator_accepts_framework_parameter(self):
|
||||
"""Test that WorkflowAutoGenerator accepts framework parameter."""
|
||||
"""Explicit framework goes straight to create() without pick_default()."""
|
||||
with patch.dict(os.environ, {'OPENAI_API_KEY': 'test-key'}):
|
||||
registry = self._fake_registry("crewai")
|
||||
generator = WorkflowAutoGenerator(
|
||||
topic="Test task",
|
||||
framework="crewai"
|
||||
framework="crewai",
|
||||
adapter_registry=registry,
|
||||
)
|
||||
assert generator.framework == "crewai"
|
||||
registry.create.assert_called_once_with("crewai")
|
||||
registry.pick_default.assert_not_called()
|
||||
|
||||
def test_workflow_generator_default_framework_is_praisonai(self):
|
||||
"""Test that default framework is praisonai."""
|
||||
"""Omitted framework resolves via the shared registry default selector."""
|
||||
with patch.dict(os.environ, {'OPENAI_API_KEY': 'test-key'}):
|
||||
generator = WorkflowAutoGenerator(topic="Test task")
|
||||
registry = self._fake_registry("praisonai", default="praisonai")
|
||||
generator = WorkflowAutoGenerator(
|
||||
topic="Test task",
|
||||
adapter_registry=registry,
|
||||
)
|
||||
assert generator.framework == "praisonai"
|
||||
registry.pick_default.assert_called_once()
|
||||
registry.create.assert_called_once_with("praisonai")
|
||||
|
||||
def test_save_workflow_respects_framework(self):
|
||||
"""Test that _save_workflow uses the specified framework."""
|
||||
with patch.dict(os.environ, {'OPENAI_API_KEY': 'test-key'}):
|
||||
generator = WorkflowAutoGenerator(
|
||||
topic="Test task",
|
||||
framework="crewai"
|
||||
framework="crewai",
|
||||
adapter_registry=self._fake_registry("crewai"),
|
||||
)
|
||||
# The framework should be stored and used in output
|
||||
assert generator.framework == "crewai"
|
||||
|
||||
Reference in new issue
Block a user