Skip to content
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

fix(backend): Add execution persistence for execution scheduler service #8649

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from

Conversation

majdyz
Copy link
Contributor

@majdyz majdyz commented Nov 14, 2024

The execution schedule is stored in the database, but the execution history is not tracked, so when the server dies, it forgot its past executions.

The UI for the agent scheduler will soon be available: #8634.

Changes 🏗️

  • Entirely removed execution schedule table, replace it with APScheduelr Postgres job store.
  • Add more information on the get_execution_schedules API.
  • This change decouples the scheduling logic with our application domain, allowing the scheduler backend to easily change in the future as we scale.
  • Change the scheduling API routes (/graph/id/schedule --> /schedule, PUT /schedule --> DELETE /schedule).
  • Add next_run_time information.
  • Allow multiple schedule per graph.

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • ...
Example test plan
  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

For configuration changes:

  • .env.example is updated or already compatible with my changes
  • docker-compose.yml is updated or already compatible with my changes
  • I have included a list of my configuration changes in the PR description (under Changes)
Examples of configuration changes
  • Changing ports
  • Adding new services that need to communicate with each other
  • Secrets or environment variable changes
  • New or infrastructure changes such as databases

@majdyz majdyz requested a review from a team as a code owner November 14, 2024 09:30
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end size/xl labels Nov 14, 2024
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns

SQL Injection:
The database URL is parsed and reconstructed manually in _extract_schema_from_url function. While urlparse is used, the function should validate/sanitize the schema parameter before using it in SQL queries to prevent potential SQL injection.

⚡ Recommended focus areas for review

Error Handling
The execute_graph function catches all exceptions but only logs them. Consider whether failed jobs should be retried or if errors should trigger notifications.

Data Validation
No validation is performed on the cron expression before adding jobs. Invalid cron expressions could cause scheduling failures.

Type Safety
The type checking for inputEntries could be more robust. Consider adding more specific type guards.

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for auto-gpt-docs ready!

Name Link
🔨 Latest commit 172045f
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/6735c3367915930008339bdb
😎 Deploy Preview https://deploy-preview-8649--auto-gpt-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 92e94e2
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67369822eca537000742ce35

@Torantulino Torantulino enabled auto-merge (squash) November 14, 2024 09:36
@majdyz majdyz disabled auto-merge November 14, 2024 09:43
@majdyz majdyz enabled auto-merge (squash) November 14, 2024 09:43
@majdyz majdyz disabled auto-merge November 14, 2024 09:43
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Nov 14, 2024
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/backend AutoGPT Platform - Back end platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 4 size/xl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant