Merge pull request #490 from jch-13/edit_prio

Support editing task priorities
This commit is contained in:
Arne Christian Beer 2024-01-15 14:16:00 +01:00 committed by GitHub
commit 3849beb63d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 112 additions and 3 deletions

View file

@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Added
- Support modification of task priorities via `pueue edit --priority/-o` and `pueue restart --edit-priority/-o` [#449](https://github.com/Nukesor/pueue/issues/449)
## [3.3.4] - unreleased
### Fixed

View file

@ -208,6 +208,10 @@ pub enum SubCommand {
/// Edit the tasks' labels before restarting.
#[arg(short = 'l', long)]
edit_label: bool,
/// Edit the tasks' priorities before restarting.
#[arg(short = 'o', long)]
edit_priority: bool,
},
#[command(about = "Either pause running tasks or specific groups of tasks.\n\
@ -270,7 +274,7 @@ pub enum SubCommand {
},
#[command(
about = "Edit the command, path or label of a stashed or queued task.\n\
about = "Edit the command, path, label, or priority of a stashed or queued task.\n\
By default only the command is edited.\n\
Multiple properties can be added in one go."
)]
@ -289,6 +293,10 @@ pub enum SubCommand {
/// Edit the task's label.
#[arg(short, long)]
label: bool,
/// Edit the task's priority.
#[arg(short = 'o', long)]
priority: bool,
},
#[command(about = "Use this to add or remove groups.\n\

View file

@ -198,6 +198,7 @@ impl Client {
command,
path,
label,
priority,
} => {
let message = edit(
&mut self.stream,
@ -206,6 +207,7 @@ impl Client {
*command,
*path,
*label,
*priority,
)
.await?;
self.handle_response(message)?;
@ -233,6 +235,7 @@ impl Client {
edit,
edit_path,
edit_label,
edit_priority,
} => {
// `not_in_place` superseeds both other configs
let in_place =
@ -249,6 +252,7 @@ impl Client {
*edit,
*edit_path,
*edit_label,
*edit_priority,
)
.await?;
Ok(true)

View file

@ -24,6 +24,7 @@ pub async fn edit(
edit_command: bool,
edit_path: bool,
edit_label: bool,
edit_priority: bool,
) -> Result<Message> {
// Request the data to edit from the server and issue a task-lock while doing so.
let init_message = Message::EditRequest(task_id);
@ -39,7 +40,7 @@ pub async fn edit(
};
// Edit the command if explicitly specified or if no flags are provided (the default)
let edit_command = edit_command || !edit_path && !edit_label;
let edit_command = edit_command || !edit_path && !edit_label && !edit_priority;
// Edit all requested properties.
let edit_result = edit_task_properties(
@ -47,9 +48,11 @@ pub async fn edit(
&init_response.command,
&init_response.path,
&init_response.label,
init_response.priority,
edit_command,
edit_path,
edit_label,
edit_priority,
);
// Any error while editing will result in the client aborting the editing process.
@ -82,6 +85,7 @@ pub async fn edit(
path: edited_props.path,
label: edited_props.label,
delete_label: edited_props.delete_label,
priority: edited_props.priority,
};
send_message(edit_message, stream).await?;
@ -94,6 +98,7 @@ pub struct EditedProperties {
pub path: Option<PathBuf>,
pub label: Option<String>,
pub delete_label: bool,
pub priority: Option<i32>,
}
/// Takes several task properties and edit them if requested.
@ -102,14 +107,17 @@ pub struct EditedProperties {
/// Fields that have been edited will be returned as their `Some(T)` equivalent.
///
/// The returned values are: `(command, path, label)`
#[allow(clippy::too_many_arguments)]
pub fn edit_task_properties(
settings: &Settings,
original_command: &str,
original_path: &Path,
original_label: &Option<String>,
original_priority: i32,
edit_command: bool,
edit_path: bool,
edit_label: bool,
edit_priority: bool,
) -> Result<EditedProperties> {
let mut props = EditedProperties::default();
@ -141,6 +149,11 @@ pub fn edit_task_properties(
};
}
// Update the priority if requested.
if edit_priority {
props.priority = Some(edit_line(settings, &original_priority.to_string())?.parse()?);
};
Ok(props)
}

View file

@ -27,6 +27,7 @@ pub async fn restart(
edit_command: bool,
edit_path: bool,
edit_label: bool,
edit_priority: bool,
) -> Result<()> {
let new_status = if stashed {
TaskStatus::Stashed { enqueue_at: None }
@ -91,9 +92,11 @@ pub async fn restart(
&task.command,
&task.path,
&task.label,
task.priority,
edit_command,
edit_path,
edit_label,
edit_priority,
)?;
// Add the tasks to the singular message, if we want to restart the tasks in-place.
@ -105,6 +108,7 @@ pub async fn restart(
path: edited_props.path,
label: edited_props.label,
delete_label: edited_props.delete_label,
priority: edited_props.priority,
});
continue;
@ -121,7 +125,7 @@ pub async fn restart(
group: task.group.clone(),
enqueue_at: None,
dependencies: Vec::new(),
priority: Some(task.priority),
priority: edited_props.priority.or(Some(task.priority)),
label: edited_props.label.or_else(|| task.label.clone()),
print_task_id: false,
};

View file

@ -26,6 +26,7 @@ pub fn edit_request(task_id: usize, state: &SharedState) -> Message {
command: task.original_command.clone(),
path: task.path.clone(),
label: task.label.clone(),
priority: task.priority,
}
.into()
}
@ -62,6 +63,10 @@ pub fn edit(message: EditMessage, state: &SharedState, settings: &Settings) -> M
} else if message.delete_label {
task.label = None;
}
// Update priority if applicable.
if let Some(priority) = message.priority {
task.priority = priority;
}
ok_or_return_failure_message!(save_state(&state, settings));

View file

@ -96,6 +96,11 @@ fn restart(
task.label = None
}
// Update priority if applicable.
if let Some(priority) = to_restart.priority {
task.priority = priority;
}
// Reset all variables of any previous run.
task.start = None;
task.end = None;

View file

@ -31,6 +31,7 @@ async fn edit_task_default() -> Result<()> {
// All other properties should be unchanged.
assert_eq!(task.path, daemon.tempdir.path());
assert_eq!(task.label, None);
assert_eq!(task.priority, 0);
Ok(())
}
@ -94,6 +95,33 @@ async fn edit_delete_label() -> Result<()> {
Ok(())
}
/// Ensure that updating the priority in the editor results in the modification of the task's priority.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn edit_change_priority() -> Result<()> {
let daemon = daemon().await?;
let shared = &daemon.settings.shared;
// Create a stashed message which we'll edit later on.
let mut message = create_add_message(shared, "this is a test");
message.stashed = true;
message.priority = Some(0);
send_message(shared, message)
.await
.context("Failed to to add stashed task.")?;
// Echo a new priority into the file.
let mut envs = HashMap::new();
envs.insert("EDITOR", "echo '99' > ");
run_client_command_with_env(shared, &["edit", "--priority", "0"], envs)?;
// Make sure that the priority has indeed been updated.
let state = get_state(shared).await?;
let task = state.tasks.get(&0).unwrap();
assert_eq!(task.priority, 99);
Ok(())
}
/// Test that automatic restoration of a task's state works, if the edit command fails for some
/// reason.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]

View file

@ -106,6 +106,36 @@ async fn restart_and_edit_task_path_and_command() -> Result<()> {
Ok(())
}
/// Test that restarting a task while editing its priority works as expected.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn restart_and_edit_task_priority() -> Result<()> {
let daemon = daemon().await?;
let shared = &daemon.settings.shared;
// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
// Set the editor to a command which replaces the temporary file's content.
let mut envs = HashMap::new();
envs.insert("EDITOR", "echo '99' > ");
// Restart the command, edit its priority and wait for it to start.
run_client_command_with_env(
shared,
&["restart", "--in-place", "--edit-priority", "0"],
envs,
)?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
// Make sure that the priority has been updated.
let state = get_state(shared).await?;
let task = state.tasks.get(&0).unwrap();
assert_eq!(task.priority, 99);
Ok(())
}
/// Test that restarting a task **not** in place works as expected.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn normal_restart_with_edit() -> Result<()> {

View file

@ -65,6 +65,7 @@ async fn test_restart_with_alias() -> Result<()> {
path: None,
label: None,
delete_label: false,
priority: Some(0),
}],
start_immediately: true,
stashed: false,

View file

@ -40,6 +40,7 @@ async fn test_edit_flow() -> Result<()> {
assert_eq!(response.task_id, 0);
assert_eq!(response.command, "ls");
assert_eq!(response.path, daemon.tempdir.path());
assert_eq!(response.priority, 0);
// Task should be locked, after the request for editing succeeded.
assert_eq!(get_task_status(shared, 0).await?, TaskStatus::Locked);
@ -57,6 +58,7 @@ async fn test_edit_flow() -> Result<()> {
path: Some("/tmp".into()),
label: Some("test".to_string()),
delete_label: false,
priority: Some(99),
},
)
.await?;
@ -68,6 +70,7 @@ async fn test_edit_flow() -> Result<()> {
assert_eq!(task.path, PathBuf::from("/tmp"));
assert_eq!(task.label, Some("test".to_string()));
assert_eq!(task.status, TaskStatus::Queued);
assert_eq!(task.priority, 99);
Ok(())
}

View file

@ -30,6 +30,7 @@ async fn test_restart_in_place() -> Result<()> {
path: Some(PathBuf::from("/tmp")),
label: Some("test".to_owned()),
delete_label: false,
priority: Some(0),
}],
start_immediately: false,
stashed: false,
@ -83,6 +84,7 @@ async fn test_cannot_restart_running() -> Result<()> {
path: None,
label: None,
delete_label: false,
priority: Some(0),
}],
start_immediately: false,
stashed: false,

View file

@ -171,6 +171,8 @@ pub struct TaskToRestart {
/// Cbor cannot represent Option<Option<T>> yet, which is why we have to utilize a
/// boolean to indicate that the label should be released, rather than an `Some(None)`.
pub delete_label: bool,
/// Restart the task with an updated priority.
pub priority: Option<i32>,
}
#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
@ -223,6 +225,7 @@ pub struct EditResponseMessage {
pub command: String,
pub path: PathBuf,
pub label: Option<String>,
pub priority: i32,
}
impl_into_message!(EditResponseMessage, Message::EditResponse);
@ -236,6 +239,7 @@ pub struct EditMessage {
/// Cbor cannot represent Option<Option<T>> yet, which is why we have to utilize a
/// boolean to indicate that the label should be released, rather than an `Some(None)`.
pub delete_label: bool,
pub priority: Option<i32>,
}
impl_into_message!(EditMessage, Message::Edit);