Skip to content

ARQ Jobs#110

Merged
alexchristy merged 48 commits intomainfrom
arq-jobs
Jul 13, 2025
Merged

ARQ Jobs#110
alexchristy merged 48 commits intomainfrom
arq-jobs

Conversation

@alexchristy
Copy link
Copy Markdown
Member

@alexchristy alexchristy commented Jul 10, 2025

Checklist

  • I have added a version label to my PR (e.g., patch, minor, major).
  • I have tested my changes locally and verified they work as expected.
  • I have added relevant tests to cover my changes.
  • I have made any necessary updates to the documentation.

Description

This PR brings ARQ async job execution functionality to the API. Background execution for long running tasks (like terraform deploy and destroy) allows the API to be snappier and more responsive. A summary of the changes is listed below.

ARQ Job:

  • Added ARQ worker and redis queue container
  • Added two new endpoints for tracking jobs
    • Get all jobs: GET /jobs
    • Get specific job: GET /jobs/{id}
  • ARQ worker now has two "jobs" functions: deploy_range and destroy_range (moved from range endpoints)
  • Move terraform dependencies to ARQ worker image
  • Added Job schemas + model to store job data

Testing:

  • Dynamically mark tests based on directory structure
  • Remove manually pytest marks in individual files
  • Added new marks slow, api, worker, and cdktf
  • Refactored deployed tests to leverage ARQ async execution (deploys one-all and multi test ranges now)

Misc:

  • Refactor base_range.py class to leverage subprocess execution for terraform commands
  • Updated terraform providers.tf to use latest AWS provider (allows docker image to build with latest provider)
  • Refactored FastAPI database dependency to allow ARQ worker to reuse database configuration

Fixes: #39

Known Issues:

  • It is possible for job records in Postgres to be stale (get stuck in a queued/in_progress state due to network disconnects and other major failures)
    • The plan is to implement a cron job that periodically queries the database for jobs that have been queued or in_progress too long (ex: >1 hour) and then query Redis again for its status and update the record in the database.
  • Job records currently grow without bound unless users manually delete records in the database
    • The plan is to implement a cron job that periodically queries for jobs that have complete/failed before a user configured cut off period (ex: 90 days for completed jobs and 180 days for failed jobs) and drop these old records from the database

…long with async parallel processing for terraform commands
… destroy to 202 since they queue up async jobs.
@alexchristy alexchristy added the minor Increment the minor version when merged label Jul 10, 2025
@alexchristy alexchristy linked an issue Jul 10, 2025 that may be closed by this pull request
@alexchristy alexchristy changed the title Arq Jobs ARQ Jobs Jul 10, 2025
@alexchristy alexchristy marked this pull request as ready for review July 10, 2025 03:36
Copilot AI review requested due to automatic review settings July 10, 2025 03:36
Copy link
Copy Markdown
Contributor

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 test suite to centralize test markers and update tests to async patterns and naming conventions. Key changes include:

  • Removed per-file pytestmark = pytest.mark.unit and direct import pytest usages in favor of dynamic marking via a pytest_collection_modifyitems hook in conftest.py.
  • Converted synchronous test functions to async def where the underlying code became coroutine-based, and added corresponding await calls.
  • Aliased cdktf.Testing to CdktfTesting in AWS stack tests to avoid import conflicts and improve clarity.

Reviewed Changes

Copilot reviewed 72 out of 73 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/.../test_*.py Removed explicit pytestmark and import pytest in unit tests; rely on dynamic marking
tests/unit/utils/test_api_utils.py Updated async patterns and removed redundant pytest imports
tests/unit/core/cdktf/stacks/test_aws_stacks.py Replaced Testing imports with Testing as CdktfTesting and updated assertions
tests/unit/core/cdktf/ranges/test_base_range.py Converted multiple test functions to async def and added await for coroutine calls
tests/conftest.py Introduced pytest_collection_modifyitems hook for dynamic marking instead of per-file markers

Copy link
Copy Markdown
Member

@Adamkadaban Adamkadaban left a comment

Choose a reason for hiding this comment

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

Looks great. At first I was wondering where all the code came from until I realized it was mostly tests lol (very you).
Good job

Comment thread src/app/core/cdktf/providers.tf
Comment thread tests/conftest.py
Copy link
Copy Markdown
Member

@Nareshp1 Nareshp1 left a comment

Choose a reason for hiding this comment

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

LGTM

@alexchristy alexchristy merged commit 0b4d1ed into main Jul 13, 2025
11 checks passed
@alexchristy alexchristy deleted the arq-jobs branch July 13, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Async background jobs

4 participants