Skip to content

Conversation

@geofmureithi
Copy link
Member

No description provided.

Copy link

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 introduces a comprehensive v1 SQLite backend implementation for the Apalis job processing framework. The implementation provides a fully-featured asynchronous job queue with task persistence, worker management, and real-time notifications.

  • Complete SQLite storage backend with migrations, configuration, and context management
  • Real-time task processing through both polling and database hook-based notifications
  • Worker lifecycle management with heartbeat tracking and orphaned task recovery

Reviewed Changes

Copilot reviewed 30 out of 36 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/lib.rs Core storage implementation with multiple backend variants and worker registration
src/sqlite_old.rs Legacy implementation retained for reference/migration purposes
src/sink.rs Task insertion sink with batched database operations
src/shared.rs Shared storage implementation for multi-worker scenarios
src/hook.rs SQLite update hook integration for real-time notifications
src/fetcher.rs Task fetching logic with polling strategies and buffering
src/context.rs Task context management for SQLite-specific metadata
src/config.rs Configuration management for polling, buffering, and worker settings
src/ack.rs Task acknowledgment handling with status calculation
src/from_row.rs Database row to task conversion utilities
queries/*.sql SQL query files for task and worker operations
migrations/*.sql Database schema migrations for jobs and workers tables
Cargo.toml Project dependencies and feature configuration
.github/workflows/*.yml Comprehensive CI/CD pipeline setup
.github/*.yml Issue templates and dependency management configuration
Files not reviewed (4)
  • .sqlx/query-0819e39ed31495e0c70a6dd549b10e5c52e64d5289ca829e2a0135a14801b930.json: Language not supported
  • .sqlx/query-0be6db0bdde33835113a74e4749bc68adca35acef6ce80e58bad3b70c3fd6d97.json: Language not supported
  • .sqlx/query-4ead10dafe0d2d024654403f289c4db57308e5cf88f44efaf23b05c585626465.json: Language not supported
  • .sqlx/query-cfc8346ce97a0f65e057b439787acaa0ee3ad0de2ad6453554e95be9df4e487e.json: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

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

Copilot reviewed 31 out of 38 changed files in this pull request and generated 5 comments.

Files not reviewed (5)
  • .sqlx/query-0be6db0bdde33835113a74e4749bc68adca35acef6ce80e58bad3b70c3fd6d97.json: Language not supported
  • .sqlx/query-1beef6dd1673e83142d565dcd7a0543bde1cbc72f692187d8a7fef06e952f513.json: Language not supported
  • .sqlx/query-4ead10dafe0d2d024654403f289c4db57308e5cf88f44efaf23b05c585626465.json: Language not supported
  • .sqlx/query-8e68049c1005dd0981469dac442cc479d318a98f4891ed46ff8588bb4e85615b.json: Language not supported
  • .sqlx/query-cfc8346ce97a0f65e057b439787acaa0ee3ad0de2ad6453554e95be9df4e487e.json: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

src/sink.rs Outdated
Comment on lines 1 to 16
use std::{
pin::Pin,
task::{Context, Poll},
};

use apalis_core::{
backend::codec::{Codec, json::JsonCodec},
error::BoxDynError,
};
use futures::Sink;
use sqlx::SqlitePool;
use ulid::Ulid;

use crate::{SqliteStorage, SqliteTask, config::Config};

type FlushFuture = Pin<Box<dyn Future<Output = Result<(), sqlx::Error>> + Send>>;
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Missing import for Future trait. The FlushFuture type alias references Future but it's not imported. Add use std::future::Future; to the imports.

Copilot uses AI. Check for mistakes.
Comment on lines +100 to +107
fn start_send(self: Pin<&mut Self>, item: SqliteTask<Args>) -> Result<(), Self::Error> {
// Add the item to the buffer
self.project()
.sink
.buffer
.push(item.try_map(|s| Encode::encode(&s).map_err(|e| sqlx::Error::Encode(e.into())))?);
Ok(())
}
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The projection is accessing a field sink that doesn't exist in the SqliteStorage struct. The buffer should be accessed directly as self.project().buffer.

Copilot uses AI. Check for mistakes.
Comment on lines +109 to +115
fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
let mut this = self.project();

// If there's no existing future and buffer is empty, we're done
if this.sink.flush_future.is_none() && this.sink.buffer.is_empty() {
return Poll::Ready(Ok(()));
}
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Similar projection issue - this.sink should be just this since the projection already gives access to the struct fields directly. This pattern continues throughout the poll_flush method.

Copilot uses AI. Check for mistakes.
src/fetcher.rs Outdated
Decode::Error: std::error::Error + Send + Sync + 'static,
Args: Send + 'static + Unpin,
Decode: Codec<Args, Compact = String> + 'static,
// Compact: Unpin + Send + 'static,
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Remove the commented out code. Dead code comments should be cleaned up in production code.

Suggested change
// Compact: Unpin + Send + 'static,

Copilot uses AI. Check for mistakes.
Comment on lines 11 to 14
reviewers:
- "mureithinjuguna"
assignees:
- "mureithinjuguna"
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

Inconsistent GitHub usernames between cargo and github-actions sections. The cargo section uses 'mureithinjuguna' while github-actions uses 'geofmureithi'. This should be consistent across both sections.

Copilot uses AI. Check for mistakes.
@geofmureithi geofmureithi merged commit 443b134 into main Oct 17, 2025
10 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants