-
Notifications
You must be signed in to change notification settings - Fork 152
Feat - implement queue #2081
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
base: master
Are you sure you want to change the base?
Feat - implement queue #2081
Changes from all commits
ec29c99
9b5d0cf
7d524ce
697814a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,6 +152,16 @@ impl FromStr for CommitType { | |
} | ||
} | ||
|
||
impl ToString for CommitType { | ||
fn to_string(&self) -> String { | ||
match self { | ||
CommitType::Try => "try", | ||
CommitType::Master => "master", | ||
} | ||
.to_string() | ||
} | ||
} | ||
|
||
#[derive(Debug, Clone, serde::Deserialize, serde::Serialize)] | ||
pub struct Commit { | ||
pub sha: String, | ||
|
@@ -791,3 +801,104 @@ pub struct ArtifactCollection { | |
pub duration: Duration, | ||
pub end_time: DateTime<Utc>, | ||
} | ||
|
||
#[derive(Debug, Clone, serde::Deserialize, serde::Serialize)] | ||
pub enum CommitJobStatus { | ||
Queued, | ||
InProgress, | ||
Finished, | ||
} | ||
|
||
impl FromStr for CommitJobStatus { | ||
type Err = String; | ||
fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
Ok(match s.to_ascii_lowercase().as_str() { | ||
"queued" => CommitJobStatus::Queued, | ||
"in_progress" => CommitJobStatus::InProgress, | ||
"finished" => CommitJobStatus::Finished, | ||
_ => return Err(format!("{} is not a valid `CommitJobStatus`", s)), | ||
}) | ||
} | ||
} | ||
|
||
impl ToString for CommitJobStatus { | ||
fn to_string(&self) -> String { | ||
match self { | ||
CommitJobStatus::Queued => "queued", | ||
CommitJobStatus::InProgress => "in_progress", | ||
CommitJobStatus::Finished => "finished", | ||
} | ||
.to_string() | ||
} | ||
} | ||
|
||
/// Represents a job in the work queue for collectors | ||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
pub struct CommitJob { | ||
pub sha: String, | ||
pub parent_sha: Option<String>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we keep the invariant that the parent SHA is always present when we insert things into the queue, then this shouldn't be nullable. |
||
pub commit_type: CommitType, | ||
pub pr: u32, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we don't implement any automatic from/to SQL row conversions and we build these structs manually anyway, I would appreciate if they were more "domain-driven" and didn't allow representing invalid states. So I would model it such that the job status looks something like this: pub enum CommitJobStatus {
Queued,
InProgress { started_at: DateTime<Utc> },
Finished { started_at: DateTime<Utc>, finished_at: DateTime<Utc> },
} The artifact data looks something like this: enum CommittType {
Try { pr: u32 },
Master { pr: u32 },
Release { label: String }
} and so on, so that on the Rust side, we can work more easily with these structs and avoid invalid states. Then in the DB layer, we'll just translate the domain structs into the corresponding atomic SQL attributes. |
||
pub commit_time: Date, | ||
pub target: Target, | ||
pub machine_id: Option<String>, | ||
pub started_at: Option<Date>, | ||
pub finished_at: Option<Date>, | ||
pub status: CommitJobStatus, | ||
pub include: Option<String>, | ||
pub exclude: Option<String>, | ||
pub runs: Option<i32>, | ||
pub backends: Option<String>, | ||
} | ||
|
||
impl CommitJob { | ||
pub fn from_db( | ||
sha: String, | ||
parent_sha: Option<String>, | ||
commit_type: CommitType, | ||
pr: u32, | ||
commit_time: Date, | ||
target: Target, | ||
machine_id: Option<String>, | ||
started_at: Option<Date>, | ||
finished_at: Option<Date>, | ||
status: CommitJobStatus, | ||
include: Option<String>, | ||
exclude: Option<String>, | ||
runs: Option<i32>, | ||
backends: Option<String>, | ||
) -> Self { | ||
Self { | ||
sha, | ||
parent_sha, | ||
commit_type, | ||
pr, | ||
commit_time, | ||
target, | ||
machine_id, | ||
started_at, | ||
finished_at, | ||
status, | ||
include, | ||
exclude, | ||
runs, | ||
backends, | ||
} | ||
} | ||
|
||
pub fn get_enqueue_column_names() -> Vec<String> { | ||
vec![ | ||
String::from("sha"), | ||
String::from("parent_sha"), | ||
String::from("commit_type"), | ||
String::from("pr"), | ||
String::from("commit_time"), | ||
String::from("status"), | ||
String::from("target"), | ||
String::from("include"), | ||
String::from("exclude"), | ||
String::from("runs"), | ||
String::from("backends"), | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
use crate::{ | ||
ArtifactCollection, ArtifactId, ArtifactIdNumber, CodegenBackend, CompileBenchmark, Target, | ||
ArtifactCollection, ArtifactId, ArtifactIdNumber, CodegenBackend, CommitJob, CompileBenchmark, | ||
Target, | ||
}; | ||
use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step}; | ||
use chrono::{DateTime, Utc}; | ||
|
@@ -178,6 +179,15 @@ pub trait Connection: Send + Sync { | |
|
||
/// Removes all data associated with the given artifact. | ||
async fn purge_artifact(&self, aid: &ArtifactId); | ||
|
||
/// Add a jobs to the queue | ||
async fn enqueue_commit_job(&self, target: Target, jobs: &[CommitJob]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the target already a part of |
||
|
||
/// Dequeue jobs | ||
async fn dequeue_commit_job(&self, machine_id: &str, target: Target) -> Option<CommitJob>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably also set the commit job to be in progress, so it should be maybe named something like "start_commit_job"? Or something like that. |
||
|
||
/// Mark the job as finished | ||
async fn finish_commit_job(&self, machine_id: &str, target: Target, sha: String) -> bool; | ||
} | ||
|
||
#[async_trait::async_trait] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not super important, but usually it's idiomatic to implement
Display
, which is more general, and then get an implementation ofToString
"for free" (there is a blanket impl ofToString
for types that implementDisplay
).