-
Notifications
You must be signed in to change notification settings - Fork 156
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 1 commit
ec29c99
9b5d0cf
7d524ce
697814a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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,110 @@ 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, | ||
} | ||
|
||
impl CommitJob { | ||
/// Create a new commit job | ||
pub fn new( | ||
sha: String, | ||
parent_sha: Option<String>, | ||
pr: u32, | ||
commit_type: CommitType, | ||
commit_time: Date, | ||
) -> Self { | ||
Self { | ||
sha, | ||
parent_sha, | ||
commit_type, | ||
pr, | ||
commit_time, | ||
status: CommitJobStatus::Queued, | ||
target: Target::X86_64UnknownLinuxGnu, | ||
machine_id: None, | ||
started_at: None, | ||
finished_at: None, | ||
} | ||
} | ||
|
||
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, | ||
) -> Self { | ||
Self { | ||
sha, | ||
parent_sha, | ||
commit_type, | ||
pr, | ||
commit_time, | ||
target, | ||
machine_id, | ||
started_at, | ||
finished_at, | ||
status, | ||
} | ||
} | ||
|
||
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"), | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
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 +178,14 @@ pub trait Connection: Send + Sync { | |
|
||
/// Removes all data associated with the given artifact. | ||
async fn purge_artifact(&self, aid: &ArtifactId); | ||
|
||
/* @Queue - Adds a job - we want to "double up" by adding one per `Target` */ | ||
/// Add a job 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 |
||
|
||
/* @Queue - currently extracts everything out of the queue as a SELECT */ | ||
/// Dequeue jobs | ||
async fn dequeue_commit_job(&self, machine_id: String, target: Target) -> Option<String>; | ||
} | ||
|
||
#[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
).