[pbs-devel] [PATCH proxmox-backup v2 1/9] server: change status of a task from a string to an enum
Dominik Csapak
d.csapak at proxmox.com
Tue Aug 11 11:57:16 CEST 2020
representing a state via an enum makes more sense in this case
we also implement FromStr and Display to make it easy to convet from/to
a string
Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
new in v2
src/api2/admin/sync.rs | 2 +-
src/api2/node/tasks.rs | 8 ++--
src/api2/types/mod.rs | 2 +-
src/server/worker_task.rs | 88 ++++++++++++++++++++++++++++++---------
4 files changed, 74 insertions(+), 26 deletions(-)
diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index f06625f6..79af03bc 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -56,7 +56,7 @@ pub fn list_sync_jobs(
if let Some(task) = last_tasks.get(&job.id) {
job.last_run_upid = Some(task.upid_str.clone());
if let Some((endtime, status)) = &task.state {
- job.last_run_state = Some(String::from(status));
+ job.last_run_state = Some(status.to_string());
job.last_run_endtime = Some(*endtime);
last = *endtime;
}
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index 6917869e..0bb043d9 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -10,7 +10,7 @@ use proxmox::{identity, list_subdirs_api_method, sortable};
use crate::tools;
use crate::api2::types::*;
-use crate::server::{self, UPID};
+use crate::server::{self, UPID, TaskState};
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
@@ -105,9 +105,9 @@ async fn get_task_status(
if crate::server::worker_is_active(&upid).await? {
result["status"] = Value::from("running");
} else {
- let exitstatus = crate::server::upid_read_status(&upid).unwrap_or(String::from("unknown"));
+ let exitstatus = crate::server::upid_read_status(&upid).unwrap_or(TaskState::Unknown);
result["status"] = Value::from("stopped");
- result["exitstatus"] = Value::from(exitstatus);
+ result["exitstatus"] = Value::from(exitstatus.to_string());
};
Ok(result)
@@ -352,7 +352,7 @@ pub fn list_tasks(
if let Some(ref state) = info.state {
if running { continue; }
- if errors && state.1 == "OK" {
+ if errors && state.1 == crate::server::TaskState::OK {
continue;
}
}
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index fb386d46..a619810d 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -595,7 +595,7 @@ impl From<crate::server::TaskListInfo> for TaskListItem {
fn from(info: crate::server::TaskListInfo) -> Self {
let (endtime, status) = info
.state
- .map_or_else(|| (None, None), |(a,b)| (Some(a), Some(b)));
+ .map_or_else(|| (None, None), |(a,b)| (Some(a), Some(b.to_string())));
TaskListItem {
upid: info.upid_str,
diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 852f3ecc..876666d6 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -11,6 +11,7 @@ use futures::*;
use lazy_static::lazy_static;
use nix::unistd::Pid;
use serde_json::{json, Value};
+use serde::{Serialize, Deserialize};
use tokio::sync::oneshot;
use proxmox::sys::linux::procfs;
@@ -190,8 +191,8 @@ pub fn create_task_log_dirs() -> Result<(), Error> {
}
/// Read exits status from task log file
-pub fn upid_read_status(upid: &UPID) -> Result<String, Error> {
- let mut status = String::from("unknown");
+pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
+ let mut status = TaskState::Unknown;
let path = upid.log_path();
@@ -212,12 +213,8 @@ pub fn upid_read_status(upid: &UPID) -> Result<String, Error> {
match iter.next() {
None => continue,
Some(rest) => {
- if rest == "OK" {
- status = String::from(rest);
- } else if rest.starts_with("WARNINGS: ") {
- status = String::from(rest);
- } else if rest.starts_with("ERROR: ") {
- status = String::from(&rest[7..]);
+ if let Ok(state) = rest.parse() {
+ status = state;
}
}
}
@@ -226,6 +223,59 @@ pub fn upid_read_status(upid: &UPID) -> Result<String, Error> {
Ok(status)
}
+/// Task State
+#[derive(Debug, PartialEq, Serialize, Deserialize)]
+pub enum TaskState {
+ /// The Task ended with an undefined state
+ Unknown,
+ /// The Task ended and there were no errors or warnings
+ OK,
+ /// The Task had 'count' amount of warnings and no errors
+ Warning { count: u64 },
+ /// The Task ended with the error described in 'message'
+ Error { message: String },
+}
+
+impl TaskState {
+ fn result_text(&self) -> String {
+ match self {
+ TaskState::Error { message } => format!("TASK ERROR: {}", message),
+ other => format!("TASK {}", other),
+ }
+ }
+}
+
+impl std::fmt::Display for TaskState {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ TaskState::Unknown => write!(f, "unknown"),
+ TaskState::OK => write!(f, "OK"),
+ TaskState::Warning { count } => write!(f, "WARNINGS: {}", count),
+ TaskState::Error { message } => write!(f, "{}", message),
+ }
+ }
+}
+
+impl std::str::FromStr for TaskState {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ if s == "unknown" {
+ Ok(TaskState::Unknown)
+ } else if s == "OK" {
+ Ok(TaskState::OK)
+ } else if s.starts_with("WARNINGS: ") {
+ let count: u64 = s[10..].parse()?;
+ Ok(TaskState::Warning{ count })
+ } else if s.len() > 0 {
+ let message = if s.starts_with("ERROR: ") { &s[7..] } else { s }.to_string();
+ Ok(TaskState::Error{ message })
+ } else {
+ bail!("unable to parse Task Status '{}'", s);
+ }
+ }
+}
+
/// Task details including parsed UPID
///
/// If there is no `state`, the task is still running.
@@ -236,9 +286,7 @@ pub struct TaskListInfo {
/// UPID string representation
pub upid_str: String,
/// Task `(endtime, status)` if already finished
- ///
- /// The `status` is either `unknown`, `OK`, `WARN`, or `ERROR: ...`
- pub state: Option<(i64, String)>, // endtime, status
+ pub state: Option<(i64, TaskState)>, // endtime, status
}
// atomically read/update the task list, update status of finished tasks
@@ -278,14 +326,14 @@ fn update_active_workers(new_upid: Option<&UPID>) -> Result<Vec<TaskListInfo>, E
None => {
println!("Detected stopped UPID {}", upid_str);
let status = upid_read_status(&upid)
- .unwrap_or_else(|_| String::from("unknown"));
+ .unwrap_or_else(|_| TaskState::Unknown);
finish_list.push(TaskListInfo {
upid, upid_str, state: Some((Local::now().timestamp(), status))
});
},
Some((endtime, status)) => {
finish_list.push(TaskListInfo {
- upid, upid_str, state: Some((endtime, status))
+ upid, upid_str, state: Some((endtime, status.parse()?))
})
}
}
@@ -503,23 +551,23 @@ impl WorkerTask {
Ok(upid_str)
}
- /// get the Text of the result
- pub fn get_log_text(&self, result: &Result<(), Error>) -> String {
-
+ /// create state from self and a result
+ pub fn create_state(&self, result: &Result<(), Error>) -> TaskState {
let warn_count = self.data.lock().unwrap().warn_count;
if let Err(err) = result {
- format!("ERROR: {}", err)
+ TaskState::Error { message: err.to_string() }
} else if warn_count > 0 {
- format!("WARNINGS: {}", warn_count)
+ TaskState::Warning { count: warn_count }
} else {
- "OK".to_string()
+ TaskState::OK
}
}
/// Log task result, remove task from running list
pub fn log_result(&self, result: &Result<(), Error>) {
- self.log(format!("TASK {}", self.get_log_text(result)));
+ let state = self.create_state(result);
+ self.log(state.result_text());
WORKER_TASK_LIST.lock().unwrap().remove(&self.upid.task_id);
let _ = update_active_workers(None);
--
2.20.1
More information about the pbs-devel
mailing list