-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: webhook and schedule triggers #3497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
LuoPengcheng12138
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
|
Hi there @LuoPengcheng12138 I have added some small updates:
|
|
@Wendong-Fan @nitpicker55555 @LuoPengcheng12138 |
fengju0213
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for awesome pr @a7m-1st ! left some comments
camel/triggers/trigger_manager.py
Outdated
| task = self._event_to_task(event) | ||
|
|
||
| # Process through workforce | ||
| result = await self.workforce.process_task_async(task) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current default routing (Workforce/ChatAgent branches) is OK as a
convenience, but it’s also a bit “baked in.” A cleaner design might be to expose a
unified handler abstraction so users decide what happens after a trigger, and the
default Workforce/ChatAgent handlers become just optional implementations.
Something like:
class TriggerEventHandler(Protocol):
async def handle(self, event: TriggerEvent) -> Any: ...TriggerManager just calls handler.handle(...)
This keeps the trigger pipeline extensible, avoids hard‑coding two paths, and makes it
easier for users to plug in their own logic. It would be great to document an example
handler interface in the docs as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, yes you are right. Although it would be one more step of configuration to the developer, but it would lift some burden from the trigger manager.
I have just updated it to have ChatAgent, Workforce, Callback and Workforce handlers. Those are made for heavy-duty where all triggers under the manager share a single instance. A user can also opt for per trigger callbacks via trigger.add_callback(my_function). In its simplest form:
from camel.triggers.handlers import ChatAgentHandler
from camel.agents import ChatAgent
agent = ChatAgent(system_message="You are a helpful assistant.")
handler = ChatAgentHandler(
chat_agent=agent, #Optional, uses open-ai by default
default_prompt="Process this trigger event: {event_payload}"
)
manager = TriggerManager(handler=handler)
await manager.register_trigger(trigger, auto_activate=True)
camel/triggers/trigger_manager.py
Outdated
| ) | ||
|
|
||
| # Process with ChatAgent | ||
| response = self.chat_agent.step(prompt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calling a sync method inside an async handler may stall the event loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right using astep I hope that is okay?
I will refactor it with the TriggerEventHandler as well
Co-authored-by: Guohao Li <lightaime@gmail.com>
|
Thanks @lightaime and @fengju0213 for your review 🙌 , I will make sure to tend to them asap. |
Description
Trigger System Core Implementation
WebhookTrigger: Complete HTTP webhook server implementation with aiohttp backend
TriggerManager: Central orchestration system for all trigger types
Slack Integration & Authentication
SlackAuth: Production-ready webhook signature verification
Slack Webhook Example: Complete integration demonstration
TODOs & Next Steps
Technical Notes
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!