Skip to content
This repository has been archived by the owner on Aug 13, 2024. It is now read-only.

Commit

Permalink
WIP #60 code cleanup/compiler nags
Browse files Browse the repository at this point in the history
  • Loading branch information
ninjabear committed Sep 28, 2016
1 parent 4c9a97c commit a9e0017
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 154 deletions.
9 changes: 1 addition & 8 deletions src/factotum/executor/execution_strategy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

#[cfg(test)]
mod tests;
use super::*;
use std::process::Command;
use chrono::UTC;
use std::time::{Instant, Duration};
Expand All @@ -30,13 +29,6 @@ pub struct RunResult {
pub return_code: i32
}

// pub struct TaskExecutionResult {
// pub name: String,
// pub attempted: bool,
// pub run_details: Option<RunResult>
// }


pub trait ExecutionStrategy {
fn execute(&self, name:&str, command:&mut Command) -> RunResult;
}
Expand All @@ -51,6 +43,7 @@ impl Simulation {

impl ExecutionStrategy for Simulation {
fn execute(&self, name:&str, command:&mut Command) -> RunResult {
info!("Simulating execution for {} with command {:?}", name, command);
RunResult {
run_started: UTC::now(),
duration: Duration::from_secs(0),
Expand Down
123 changes: 0 additions & 123 deletions src/factotum/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@
use factotum::factfile::Task as FactfileTask;
use factotum::factfile::Factfile;
use std::process::Command;
use std::time::{Instant, Duration};
use std::thread;
use std::sync::mpsc;
use std::collections::HashMap;
use chrono::DateTime;
use chrono::UTC;

pub mod execution_strategy;
pub mod task_list;
Expand All @@ -31,34 +27,6 @@ mod tests;
use factotum::executor::task_list::*;
use factotum::executor::execution_strategy::*;

enum TaskResult {
Ok(Task<FactfileTask>),
StopDependantTasks(Task<FactfileTask>),
Error(Task<FactfileTask>)
}


// pub enum ExecutionResult {
// AllTasksComplete(Vec<TaskExecutionResult>),
// EarlyFinishOk(Vec<TaskExecutionResult>),
// AbnormalTermination(Vec<TaskExecutionResult>)
// }

// #[inline]
// fn drain_values(mut map:HashMap<String, TaskExecutionResult>, tasks_in_order:&Vec<Vec<&FactfileTask>>) -> Vec<TaskExecutionResult> {
// let mut task_seq:Vec<TaskExecutionResult> = vec![];
// for task_level in tasks_in_order.iter() {
// for task in task_level.iter() {
// match map.remove(&task.name) {
// Some(task_result) => task_seq.push(task_result),
// _ => warn!("A task ({}) does not have an execution result? Skipping", task.name)
// }
// }
// }
// task_seq
// }


pub fn get_task_execution_list(factfile:&Factfile, start_from:Option<String>) -> TaskList<&FactfileTask> {
let mut task_list = TaskList::<&FactfileTask>::new();

Expand Down Expand Up @@ -110,11 +78,9 @@ pub fn execute_factfile(factfile:&Factfile, start_from:Option<String>) -> TaskLi
{
let tx = tx.clone();
let args = format_args(&task.task_spec.command, &task.task_spec.arguments);
let executor = task.task_spec.executor.to_string();
let task_name = task.name.to_string();

thread::spawn(move || {
let start_time = UTC::now();
let mut command = Command::new("sh");
command.arg("-c");
command.arg(args);
Expand Down Expand Up @@ -157,55 +123,6 @@ pub fn execute_factfile(factfile:&Factfile, start_from:Option<String>) -> TaskLi
}

task_group[idx].run_result = Some(task_result);


// if terminate_job_codes.contains(&return_code) {
// (TaskResult::TerminateJobPlease(return_code, run_duration), task_stdout_opt, task_stderr_opt)
// } else if continue_job_codes.contains(&return_code) {
// (TaskResult::Ok(return_code, run_duration), task_stdout_opt, task_stderr_opt)
// } else {
// let expected_codes = continue_job_codes.iter()
// .map(|code| code.to_string())
// .collect::<Vec<String>>()
// .join(",");
// (TaskResult::Error(Some(return_code), ),
// task_stdout_opt,
// task_stderr_opt)
// }

// (idx, TaskResult::Error(code, msg), stdout, stderr, start_time) => {
// warn!("task '{}' failed to execute!\n{}", task_group[idx].name, msg);
// let task_result:&mut TaskExecutionResult = task_results.get_mut(&task_group[idx].name).unwrap();
// task_result.attempted = true;

// if let Some(return_code) = code {
// task_result.run_details = Some(RunResult {
// run_started: start_time,
// duration: Duration::from_secs(0),
// requests_job_termination: false,
// task_execution_error: Some(msg),
// stdout: stdout,
// stderr: stderr,
// return_code: return_code });
// }
// task_failed = true;
// },
// (idx, TaskResult::TerminateJobPlease(code, duration), stdout, stderr, start_time) => {
// warn!("job will stop as task '{}' called for termination (no-op) with code {}", task_level[idx].name, code);

// let task_result:&mut TaskExecutionResult = task_results.get_mut(&task_group[idx].name).unwrap();
// task_result.attempted = true;
// task_result.run_details = Some(RunResult {
// run_started: start_time,
// duration: duration,
// requests_job_termination: true,
// task_execution_error: None,
// stdout: stdout,
// stderr: stderr,
// return_code: code });

// terminate_job_please = true;
// }
}

if terminate_job_please || task_failed {
Expand All @@ -217,46 +134,6 @@ pub fn execute_factfile(factfile:&Factfile, start_from:Option<String>) -> TaskLi
tasklist
}

// fn execute_task(task_name:String, executor:String, args:String, terminate_job_codes:Vec<i32>, continue_job_codes:Vec<i32>) -> (TaskResult, Option<String>, Option<String>) {
// if executor!="shell" {
// return (TaskResult::Error(None, "Only shell executions are supported currently!".to_string()), None, None)
// } else {
// let run_start = Instant::now();
// info!("Executing sh -c {:?}", args);
// match Command::new("sh").arg("-c").arg(args).output() {
// Ok(r) => {
// let run_duration = run_start.elapsed();
// let return_code = r.status.code().unwrap_or(1); // 1 will be returned if the process was killed by a signal

// let task_stdout: String = String::from_utf8_lossy(&r.stdout).trim_right().into();
// let task_stderr: String = String::from_utf8_lossy(&r.stderr).trim_right().into();

// info!("task '{}' stdout:\n'{}'", task_name, task_stdout);
// info!("task '{}' stderr:\n'{}'", task_name, task_stderr);

// let task_stdout_opt = if task_stdout.is_empty() { None } else { Some(task_stdout) };
// let task_stderr_opt = if task_stderr.is_empty() { None } else { Some(task_stderr) };

// if terminate_job_codes.contains(&return_code) {
// (TaskResult::TerminateJobPlease(return_code, run_duration), task_stdout_opt, task_stderr_opt)
// } else if continue_job_codes.contains(&return_code) {
// (TaskResult::Ok(return_code, run_duration), task_stdout_opt, task_stderr_opt)
// } else {
// let expected_codes = continue_job_codes.iter()
// .map(|code| code.to_string())
// .collect::<Vec<String>>()
// .join(",");
// (TaskResult::Error(Some(return_code), format!("the task exited with a value not specified in continue_job - {} (task expects one of the following return codes to continue [{}])", return_code, expected_codes)),
// task_stdout_opt,
// task_stderr_opt)
// }

// },
// Err(message) => (TaskResult::Error(None, format!("Error executing process - {}", message)), None, None)
// }
// }
// }

pub fn format_args(command:&str, args:&Vec<String>) -> String {
let arg_str = args.iter()
.map(|s| format!("\"{}\"", s))
Expand Down
15 changes: 0 additions & 15 deletions src/factotum/executor/task_list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,9 @@

#[cfg(test)]
mod tests;
use chrono::DateTime;
use chrono::UTC;
use std::time::Duration;
use std::collections::HashMap;
use factotum::executor::execution_strategy::RunResult;

// #[derive(Clone)]
// pub struct RunResult {
// pub run_started: DateTime<UTC>,
// pub duration: Duration,
// pub requests_job_termination: bool,
// pub task_execution_error: Option<String>,
// pub stdout: Option<String>,
// pub stderr: Option<String>,
// pub return_code: i32
// }

#[derive(Clone, PartialEq, Debug)]
pub enum State {
WAITING,
Expand Down Expand Up @@ -145,6 +131,5 @@ impl<T> TaskList<T> {
return seen;
}


}

1 change: 0 additions & 1 deletion src/factotum/executor/task_list/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

use super::*;


#[test]
fn task_new_defaults_good() {
let task = Task::<String>::new("hello", "world".to_string());
Expand Down
4 changes: 3 additions & 1 deletion src/factotum/executor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,6 @@ fn get_formatted_args() {
"world".to_string(),
"abc abc".to_string()]);
assert_eq!(args_list, "echo \"hello\" \"world\" \"abc abc\"");
}
}

// todo write test for rejecting non "shell" execution types
4 changes: 3 additions & 1 deletion src/factotum/parser/schemavalidator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@
* implied. See the Apache License Version 2.0 for the specific language
* governing permissions and limitations there under.
*/



// TODO write some tests for the schema validator
15 changes: 15 additions & 0 deletions src/factotum/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
/*
* Copyright (c) 2016 Snowplow Analytics Ltd. All rights reserved.
*
* This program is licensed to you under the Apache License Version 2.0, and
* you may not use this file except in compliance with the Apache License
* Version 2.0. You may obtain a copy of the Apache License Version 2.0 at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the Apache License Version 2.0 is distributed on an "AS
* IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
* implied. See the Apache License Version 2.0 for the specific language
* governing permissions and limitations there under.
*/

use factotum::factfile::Task;
use factotum::factfile::OnResult;

Expand Down
8 changes: 3 additions & 5 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ extern crate chrono;

use docopt::Docopt;
use std::fs;
use factotum::executor::task_list::{Task, TaskList, State};
use factotum::executor::execution_strategy::{RunResult};
use factotum::executor::task_list::{Task, State};
use factotum::factfile::Factfile;
use factotum::factfile::{Task as FactfileTask};
use colored::*;
Expand Down Expand Up @@ -419,7 +418,7 @@ fn get_duration_with_hours() {
fn test_get_task_result_line_str() {
use chrono::UTC;
use factotum::executor::execution_strategy::RunResult;
use factotum::factfile::{ Factfile, Task as FactfileTask, OnResult };
use factotum::factfile::{ Task as FactfileTask, OnResult };

// successful after 20 secs
let dt = UTC::now();
Expand Down Expand Up @@ -532,14 +531,13 @@ fn test_get_task_result_line_str() {
assert_eq!(expected_failed, stdout_failed);
assert_eq!(format!("Task '{}' stderr:\n{}\n", "fails".cyan(), "There's errors".red()), stderr_failed.unwrap());

// todo noop ?
}

#[test]
fn test_get_task_results_str_summary() {
use chrono::UTC;
use factotum::executor::execution_strategy::RunResult;
use factotum::factfile::{Factfile, Task as FactfileTask, OnResult};
use factotum::factfile::{Task as FactfileTask, OnResult};

let dt = UTC::now();

Expand Down

0 comments on commit a9e0017

Please sign in to comment.