Final changes for trigger beta 0.0.3b1 #435
Conversation
- Updated README.md to include new "Triggers" feature for automatic scheduling of workflows using cron expressions. - Added "Triggers" section in mkdocs.yml for navigation. - Expanded concepts documentation to explain triggers and their lifecycle. - Introduced examples for implementing triggers in graph templates and Python SDK. - Created a new triggers.md file detailing trigger usage and common cron expressions. - Updated StateManager and related models to support triggers for automatic graph execution.
- Revised the description of the `triggers` parameter in the README to indicate its beta status and specify the SDK version (0.0.3b1) associated with this feature. This enhances clarity for users regarding the current state of the triggers functionality.
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App. |
|
Warning Rate limit exceeded@NiveditJain has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds cron-based Triggers (beta) across docs, SDK, and state-manager: documents new Triggers concept and examples; exports Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant SDK as Python SDK (StateManager)
participant API as Exosphere Host API
participant DB as Scheduler DB
participant Exec as Execution Engine
Dev->>SDK: upsert_graph(name,nodes,..., triggers=[CronTrigger(expr)...])
SDK->>API: POST /graphs { ..., triggers:[{type:"CRON", expression:"..."}] }
API->>DB: register cron trigger records
DB-->>API: ack triggers created
API-->>SDK: 200 OK { graph_id, triggers_registered:true }
rect rgba(220,245,230,0.35)
Note over DB,API: At scheduled times
DB->>API: fire trigger (graph_id, trigger_id)
API->>Exec: start graph execution
Exec-->>API: execution status
end
alt Error registering triggers
API-->>SDK: 4xx/5xx
SDK-->>Dev: propagate error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Summary of ChangesHello @NiveditJain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request finalizes the beta release of the 'Triggers' feature, enabling users to schedule the automatic execution of their Exosphere graphs using cron expressions. The changes primarily focus on updating documentation across the project, including the main README, concept guides, and Python SDK examples, to clearly explain how to define, configure, and utilize these new scheduling capabilities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Triggers' feature for automatic graph execution via cron scheduling. The changes primarily involve adding extensive documentation for this new beta feature, including a new triggers.md page, and updating existing documentation and Python SDK examples. The backend implementation for handling cron triggers is also included. My review focuses on the clarity and correctness of the new documentation. I've pointed out a few areas in the documentation examples that could be improved for better user understanding, such as incomplete code snippets and a minor syntax error in a JSON example.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python-sdk/exospherehost/statemanager.py (1)
128-181: Avoid positional-argument breakage inupsert_graph.Dropping
triggersin ahead ofvalidation_timeoutshifts every downstream positional argument, so existing calls such asupsert_graph(..., retry_policy, store_config, 120, 2)now inject120intotriggersand explode at runtime. Please keep the original positional contract by appending the new parameter instead. For example:- async def upsert_graph(self, graph_name: str, graph_nodes: list[GraphNodeModel], secrets: dict[str, str], retry_policy: RetryPolicyModel | None = None, store_config: StoreConfigModel | None = None, triggers: list[CronTrigger] | None = None, validation_timeout: int = 60, polling_interval: int = 1): + async def upsert_graph(self, graph_name: str, graph_nodes: list[GraphNodeModel], secrets: dict[str, str], retry_policy: RetryPolicyModel | None = None, store_config: StoreConfigModel | None = None, validation_timeout: int = 60, polling_interval: int = 1, triggers: list[CronTrigger] | None = None): @@ - triggers (list[CronTrigger] | None): Optional list of cron triggers for automatic - graph execution. Each trigger contains a cron expression that schedules when - the graph should be executed automatically. + triggers (list[CronTrigger] | None): Optional list of cron triggers for automatic + graph execution. Each trigger contains a cron expression that schedules when + the graph should be executed automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
README.md(2 hunks)docs/docs/exosphere/concepts.md(3 hunks)docs/docs/exosphere/create-graph.md(1 hunks)docs/docs/exosphere/graph-components.md(1 hunks)docs/docs/exosphere/python-sdk-graph.md(2 hunks)docs/docs/exosphere/triggers.md(1 hunks)docs/mkdocs.yml(1 hunks)python-sdk/README.md(4 hunks)python-sdk/exospherehost/models.py(1 hunks)python-sdk/exospherehost/statemanager.py(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/docs/exosphere/concepts.md
56-56: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
91-91: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
README.md
31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
91-91: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
docs/docs/exosphere/triggers.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
210-210: Files should end with a single newline character
(MD047, single-trailing-newline)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…ate processing - Updated the `upsert_graph_template` function to delete pending cron triggers when old triggers are present, enhancing trigger management. - Modified the `create_next_triggers` function in `trigger_cron.py` to handle duplicate key errors gracefully, ensuring robust trigger creation. - Streamlined the `verify_graph` function by removing the old triggers parameter and directly creating new cron triggers, improving clarity and performance. - Enhanced error handling in the trigger processing logic to log errors and prevent failures from interrupting the workflow.
coderabbitai
bot
added
documentation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Triggers' feature for automatic cron-based scheduling of graphs. The changes are comprehensive, covering documentation, the Python SDK, and backend logic in the state-manager. The implementation is generally solid, particularly the reliability improvements in trigger execution. However, I've identified a couple of areas for improvement. There's a potential race condition in the trigger update logic that could affect correctness, and a performance concern with the catch-up mechanism for missed triggers. I've also suggested a minor clarification in the new documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
state-manager/app/tasks/verify_graph.py (1)
104-155: Restore cron cancellation contract and verify_graph signatureDropping
cancel_cronsand narrowingverify_graphto a single parameter removed the only path that clears existing pending cron triggers when verification runs. Callers (including the unit tests) still import/patchcancel_cronsand invokeverify_graphwith the previous(graph_template, old_triggers)signature, which is why the pipeline now fails withAttributeErrorandTypeError. It also leaves any direct invocations ofverify_graphunable to cancel stale cron triggers, so duplicates accumulate.Please reinstate
cancel_cronsand keep the old signature (e.g.old_triggers: list[Trigger] | None = None) so we cancel when old triggers are supplied before creating new ones. The helper can reuse the existing deletion logic introduced in the upsert flow. Doing so also resolves the Ruff F401 warnings becauseTriggerandInbecome used again.+async def cancel_crons(graph_template: GraphTemplate, old_triggers: list[Trigger]): + expressions_to_cancel = [ + trigger.value["expression"] + for trigger in old_triggers + if trigger.type == TriggerTypeEnum.CRON + ] + if not expressions_to_cancel: + return + + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == graph_template.name, + DatabaseTriggers.namespace == graph_template.namespace, + DatabaseTriggers.trigger_status == TriggerStatusEnum.PENDING, + DatabaseTriggers.type == TriggerTypeEnum.CRON, + In(DatabaseTriggers.expression, expressions_to_cancel) + ).delete_many() + -async def verify_graph(graph_template: GraphTemplate): +async def verify_graph( + graph_template: GraphTemplate, + old_triggers: list[Trigger] | None = None, +): @@ - await graph_template.save() - await create_crons(graph_template) + await graph_template.save() + if old_triggers: + await cancel_crons(graph_template, old_triggers) + await create_crons(graph_template)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
docs/docs/exosphere/graph-components.md(1 hunks)state-manager/app/controller/upsert_graph_template.py(2 hunks)state-manager/app/tasks/trigger_cron.py(3 hunks)state-manager/app/tasks/verify_graph.py(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#428
File: state-manager/app/tasks/verify_graph.py:4-5
Timestamp: 2025-09-28T13:35:42.851Z
Learning: In the cron trigger scheduling logic for state-manager/app/tasks/verify_graph.py, the system intentionally schedules at least one trigger beyond the trigger_ahead_time window to ensure continuity of scheduled executions. The current logic of appending an event then breaking is by design to guarantee "at least one next" trigger.
📚 Learning: 2025-09-28T13:35:42.851Z
Learnt from: NiveditJain
PR: exospherehost/exospherehost#428
File: state-manager/app/tasks/verify_graph.py:4-5
Timestamp: 2025-09-28T13:35:42.851Z
Learning: In the cron trigger scheduling logic for state-manager/app/tasks/verify_graph.py, the system intentionally schedules at least one trigger beyond the trigger_ahead_time window to ensure continuity of scheduled executions. The current logic of appending an event then breaking is by design to guarantee "at least one next" trigger.
Applied to files:
state-manager/app/tasks/trigger_cron.pystate-manager/app/tasks/verify_graph.py
🧬 Code graph analysis (3)
state-manager/app/tasks/trigger_cron.py (2)
state-manager/app/models/db/trigger.py (1)
DatabaseTriggers(9-36)state-manager/app/models/trigger_models.py (2)
TriggerTypeEnum(6-7)TriggerStatusEnum(9-14)
state-manager/app/tasks/verify_graph.py (2)
state-manager/app/models/db/graph_template_model.py (1)
GraphTemplate(18-347)state-manager/app/models/trigger_models.py (1)
TriggerTypeEnum(6-7)
state-manager/app/controller/upsert_graph_template.py (3)
state-manager/app/models/db/trigger.py (1)
DatabaseTriggers(9-36)state-manager/app/models/trigger_models.py (2)
TriggerStatusEnum(9-14)TriggerTypeEnum(6-7)state-manager/app/tasks/verify_graph.py (1)
verify_graph(129-161)
🪛 GitHub Actions: Ruff check on changed files only
state-manager/app/tasks/verify_graph.py
[error] 5-5: Ruff: F401 'In' from beanie.operators is imported but unused. Remove unused import (lint error).
[error] 12-12: Ruff: F401 'Trigger' from app.models.trigger_models is imported but unused. Remove unused import (lint error).
🪛 GitHub Actions: State Manager Unit Tests
state-manager/app/tasks/verify_graph.py
[error] 1-1: AttributeError: module 'app.tasks.verify_graph' does not have the attribute 'cancel_crons'. Tests patch this symbol but it is missing in the module.
[error] 1-1: TypeError: verify_graph() takes 1 positional argument but 2 were given.
🔇 Additional comments (1)
docs/docs/exosphere/graph-components.md (1)
107-123: Triggers JSON example looks correct.Thanks for tightening the snippet—this JSON is now standards-compliant and matches the documented cron trigger schema.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Updated the `create_next_triggers` function in `trigger_cron.py` to raise exceptions on errors, enhancing error visibility. - Modified test cases in `test_upsert_graph_template.py` and `test_verify_graph.py` to remove unnecessary parameters from `verify_graph`, streamlining the function calls and improving clarity. - Adjusted mock setups in tests to reflect the changes in function signatures, ensuring accurate testing of trigger functionality.
…n/exospherehost into triggers-finalization
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/docs/exosphere/triggers.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#428
File: state-manager/app/tasks/verify_graph.py:4-5
Timestamp: 2025-09-28T13:35:42.851Z
Learning: In the cron trigger scheduling logic for state-manager/app/tasks/verify_graph.py, the system intentionally schedules at least one trigger beyond the trigger_ahead_time window to ensure continuity of scheduled executions. The current logic of appending an event then breaking is by design to guarantee "at least one next" trigger.
🪛 markdownlint-cli2 (0.18.1)
docs/docs/exosphere/triggers.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
211-211: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (3)
docs/docs/exosphere/triggers.md (3)
172-192: Good fix: the retry example now clarifies assumed variables.The added comment removes ambiguity and makes the snippet self‑contained enough in context.
83-90: No revisions needed: imports andupsert_graphsignature are accurate. All items are correctly exported at the package root, and thetriggersandretry_policyparameters match the implementation.
161-163: Confirm cron timezone and DOW support: No timezone config found in code—verify APScheduler’s default evaluation timezone (it may not be UTC) and croniter’s support for both 0 and 7 as Sunday. Update docs if actual behavior differs.
- Removed unused import of `In` from beanie.operators to clean up the code. - Streamlined the import section by retaining only necessary components, enhancing readability.
1 participant
No description provided.