Re: [pbs-devel] [PATCH proxmox 1/7] rest-server: add function that returns a join handle for spawn

[Christian Ebner <c.ebner@proxmox.com>]

nit inline

On 1/16/25 07:45, Hannes Laimer wrote:
> We need this handle when we want to start multiple jobs sequentially
> from a 'manage' task. With the handle we can avoid polling for the
> task status with its upid.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   proxmox-rest-server/src/worker_task.rs | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
> index beec691e..020bdb95 100644
> --- a/proxmox-rest-server/src/worker_task.rs
> +++ b/proxmox-rest-server/src/worker_task.rs
> @@ -14,6 +14,7 @@ use serde::{Deserialize, Serialize};
>   use serde_json::{json, Value};
>   use tokio::signal::unix::SignalKind;
>   use tokio::sync::{oneshot, watch};
> +use tokio::task::JoinHandle;
>   use tracing::{info, warn};
>   
>   use proxmox_daemon::command_socket::CommandSocket;
> @@ -942,6 +943,22 @@ impl WorkerTask {
>           to_stdout: bool,
>           f: F,
>       ) -> Result<String, Error>
> +    where
> +        F: Send + 'static + FnOnce(Arc<WorkerTask>) -> T,
> +        T: Send + 'static + Future<Output = Result<(), Error>>,
> +    {
> +        let (upid_str, _) = Self::spawn_with_handle(worker_type, worker_id, auth_id, to_stdout, f)?;
> +        Ok(upid_str)
> +    }
> +

nit: docstring for `spawn_with_handle` should primarily tell what it 
does, only secondarily what it is used for.

Maybe something like:
```
/// Spawn a new worker task with log context like `spawn`, but return 
the tasks join handle in addition to its upid.
/// Allows for a management tasks to handle multiple worker tasks.
```

> +    /// Needed when a 'management' task starts multiple tasks sequentially
> +    pub fn spawn_with_handle<F, T>(
> +        worker_type: &str,
> +        worker_id: Option<String>,
> +        auth_id: String,
> +        to_stdout: bool,
> +        f: F,
> +    ) -> Result<(String, JoinHandle<()>), Error>
>       where
>           F: Send + 'static + FnOnce(Arc<WorkerTask>) -> T,
>           T: Send + 'static + Future<Output = Result<(), Error>>,
> @@ -950,12 +967,12 @@ impl WorkerTask {
>           let upid_str = worker.upid.to_string();
>           let f = f(worker.clone());
>   
> -        tokio::spawn(LogContext::new(logger).scope(async move {
> +        let handle = tokio::spawn(LogContext::new(logger).scope(async move {
>               let result = f.await;
>               worker.log_result(&result);
>           }));
>   
> -        Ok(upid_str)
> +        Ok((upid_str, handle))
>       }
>   
>       /// Create a new worker thread.



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel