Skip to content

Refactor task execution workers #2178

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 2, 2025
Merged

Refactor task execution workers #2178

merged 7 commits into from
Aug 2, 2025

Conversation

zurdi15
Copy link
Member

@zurdi15 zurdi15 commented Jul 31, 2025 •
edited
Loading

Description
This PR changes the tasks execution to be executed in the valkey worker to avoid timeout issues.

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

@zurdi15 zurdi15 self-assigned this Jul 31, 2025
@trunk-io Trunk.io
Copy link

trunk-io bot commented Jul 31, 2025

Running Code Quality on PRs by uploading data to Trunk will soon be removed. You can still run checks on your PRs using trunk-action - see the migration guide for more information.

Copilot

This comment was marked as outdated.

Sign in to view

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 31, 2025 •
edited
Loading

Test Results

543 tests   - 3   542 ✅  - 3   1m 0s ⏱️ -1s
  1 suites ±0     1 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit d8af2f8. ± Comparison against base commit 5f0e626.

This pull request removes 19 and adds 16 tests. Note that renamed tests count towards both. 
backend.endpoints.tests.test_tasks.TestGetAvailableTasks ‑ test_get_available_tasks_import_error
backend.endpoints.tests.test_tasks.TestGetAvailableTasks ‑ test_get_available_tasks_invalid_task_object
backend.endpoints.tests.test_tasks.TestGetAvailableTasks ‑ test_get_available_tasks_no_valid_tasks
backend.endpoints.tests.test_tasks.TestGetAvailableTasks ‑ test_get_available_tasks_non_callable_run
backend.endpoints.tests.test_tasks.TestGetAvailableTasks ‑ test_get_available_tasks_success
backend.endpoints.tests.test_tasks.TestTasksEndpoints ‑ test_list_tasks_empty
backend.endpoints.tests.test_tasks.TestTasksEndpoints ‑ test_list_tasks_success
backend.endpoints.tests.test_tasks.TestTasksEndpoints ‑ test_list_tasks_unauthorized
backend.endpoints.tests.test_tasks.TestTasksEndpoints ‑ test_run_all_tasks_no_runnable_tasks
backend.endpoints.tests.test_tasks.TestTasksEndpoints ‑ test_run_all_tasks_no_tasks
…

backend.endpoints.tests.test_tasks.TestIntegration ‑ test_error_handling
backend.endpoints.tests.test_tasks.TestIntegration ‑ test_full_workflow
backend.endpoints.tests.test_tasks.TestListTasks ‑ test_list_tasks_empty
backend.endpoints.tests.test_tasks.TestListTasks ‑ test_list_tasks_insufficient_scope
backend.endpoints.tests.test_tasks.TestListTasks ‑ test_list_tasks_success
backend.endpoints.tests.test_tasks.TestListTasks ‑ test_list_tasks_unauthorized
backend.endpoints.tests.test_tasks.TestRunAllTasks ‑ test_run_all_tasks_mixed_conditions
backend.endpoints.tests.test_tasks.TestRunAllTasks ‑ test_run_all_tasks_no_runnable_tasks
backend.endpoints.tests.test_tasks.TestRunAllTasks ‑ test_run_all_tasks_success
backend.endpoints.tests.test_tasks.TestRunAllTasks ‑ test_run_all_tasks_unauthorized
…

♻️ This comment has been updated with latest results.

@zurdi15 zurdi15 changed the title refactor: Rask execution on worker refactor: Task execution on worker Jul 31, 2025
@gantoine gantoine changed the title refactor: Task execution on worker Refactor task execution workers Aug 2, 2025
@gantoine gantoine force-pushed the fix/run-tasks-in-worker branch from 0f37127 to 036a66a Compare August 2, 2025 14:34
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2025 •
edited
Loading

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
8711 6135 70% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
backend/endpoints/platform.py 43% 🟢
backend/endpoints/responses/tasks.py 100% 🟢
backend/endpoints/tasks.py 100% 🟢
backend/tasks/_init_.py 100% 🟢
backend/tasks/manual/cleanup_orphaned_resources.py 27% 🟢
backend/tasks/scheduled/scan_library.py 89% 🟢
backend/tasks/scheduled/update_launchbox_metadata.py 98% 🟢
backend/tasks/scheduled/update_switch_titledb.py 100% 🟢
backend/tasks/tasks.py 100% 🟢
TOTAL 84% 🟢

updated for commit: d8af2f8 by action🐍

@gantoine gantoine requested a review from Copilot August 2, 2025 14:36
Copilot
Copilot AI reviewed Aug 2, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the task execution system to use a Valkey worker queue for task execution instead of direct execution, which helps avoid timeout issues. The refactoring also introduces a cleaner task inheritance hierarchy and removes the dynamic task discovery mechanism in favor of explicitly defined task registrations.

  • Refactored task execution to use low_prio_queue.enqueue() instead of direct await task.run() calls
  • Introduced a base Task class with PeriodicTask inheriting from it for better type structure
  • Replaced dynamic task discovery with explicit task registration dictionaries
  • Updated task constructors to accept title as a required parameter and moved task-specific attributes to constructor arguments

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
frontend/src/views/Home.vue Added loading state template with progress indicator for better UX
frontend/src/components/Settings/Administration/Tasks.vue Cleaned up task mapping logic and updated type imports from TaskInfoDict to TaskInfo
backend/tasks/tests/test_tasks.py Updated test fixtures to include required title parameter in task constructors
backend/tasks/tasks.py Introduced base Task class and refactored PeriodicTask to inherit from it with cleaner constructor signature
backend/tasks/scheduled/*.py Updated task constructors to use new parameter order with title and manual_run as constructor arguments
backend/tasks/manual/cleanup_orphaned_resources.py Converted to inherit from new Task base class with proper constructor
backend/endpoints/tests/test_tasks.py Completely rewrote tests to use explicit task mocking instead of dynamic discovery testing
backend/endpoints/tasks.py Replaced dynamic task discovery with explicit task dictionaries and updated execution to use worker queue
backend/endpoints/responses/tasks.py Renamed TaskInfoDict to TaskInfo for consistency
backend/endpoints/platform.py Removed unused properties from platform data structure
Files not reviewed (2)
  • frontend/src/generated/index.ts: Language not supported
  • frontend/src/generated/models/TaskInfo.ts: Language not supported

@gantoine gantoine force-pushed the fix/run-tasks-in-worker branch from 9a07937 to d8af2f8 Compare August 2, 2025 16:56
@gantoine gantoine merged commit a9bc4ad into master Aug 2, 2025
10 checks passed
@gantoine gantoine deleted the fix/run-tasks-in-worker branch August 2, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@adamantike adamantike Awaiting requested review from adamantike

@gantoine gantoine Awaiting requested review from gantoine

Assignees

@zurdi15 zurdi15

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zurdi15 @gantoine