all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Proxmox Datacenter Manager development discussion"
	<pdm-devel@lists.proxmox.com>, "Shan Shaji" <s.shaji@proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager 1/3] cli: admin: make cli handling async
Date: Tue, 27 Jan 2026 10:17:18 +0100	[thread overview]
Message-ID: <DFZ9BS9TCBLM.3GTYR8FR24RZ6@proxmox.com> (raw)
In-Reply-To: <20260123172910.244121-2-s.shaji@proxmox.com>

Hi Shan, thanks for these patches!

Some comments inline.

On Fri Jan 23, 2026 at 6:29 PM CET, Shan Shaji wrote:
> The acme API methods internally create workers to process API requests.
> An error was thrown if the methods invoked without initializing the
> worker tasks.
>
> Since `init_worker_tasks` needs to be called from an async runtime.
> Moved the content of the main function to async `run` function and
> wrapped using `proxmox_async::runtime::main`, which creates a Tokio
> runtime.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  cli/admin/Cargo.toml  |  3 +++
>  cli/admin/src/main.rs | 51 ++++++++++++++++++++++++++-----------------
>  2 files changed, 34 insertions(+), 20 deletions(-)
>
> diff --git a/cli/admin/Cargo.toml b/cli/admin/Cargo.toml
> index 0dec423..e566b39 100644
> --- a/cli/admin/Cargo.toml
> +++ b/cli/admin/Cargo.toml
> @@ -19,6 +19,9 @@ proxmox-product-config.workspace = true
>  proxmox-router = { workspace = true, features = [ "cli" ], default-features = false }
>  proxmox-schema = { workspace = true, features = [ "api-macro" ] }
>  proxmox-access-control.workspace = true
> +proxmox-rest-server.workspace = true
> +proxmox-sys.workspace = true
> +proxmox-daemon.workspace = true
>  
>  pdm-api-types.workspace = true
>  pdm-config.workspace = true
> diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
> index f698fa2..bcaa0a6 100644
> --- a/cli/admin/src/main.rs
> +++ b/cli/admin/src/main.rs
> @@ -1,35 +1,31 @@
> +use anyhow::Error;
>  use serde_json::{json, Value};
>  
>  use proxmox_router::cli::{
> -    default_table_format_options, format_and_print_result_full, get_output_format, run_cli_command,
> -    CliCommand, CliCommandMap, CliEnvironment, ColumnConfig, OUTPUT_FORMAT,
> +    default_table_format_options, format_and_print_result_full, get_output_format,
> +    run_async_cli_command, CliCommand, CliCommandMap, CliEnvironment, ColumnConfig, OUTPUT_FORMAT,
>  };
>  use proxmox_router::RpcEnvironment;
> -
>  use proxmox_schema::api;
> +use proxmox_sys::fs::CreateOptions;
>  
>  mod remotes;
>  mod support_status;
>  
> -fn main() {
> -    //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to proxmox-sys and uncomment
> -
> -    let api_user = pdm_config::api_user().expect("cannot get api user");
> -    let priv_user = pdm_config::priv_user().expect("cannot get privileged user");
> -    proxmox_product_config::init(api_user, priv_user);
> +async fn run() -> Result<(), Error> {
> +    let api_user = pdm_config::api_user()?;
> +    let priv_user = pdm_config::priv_user()?;

if you convert these .expect("...") calls to errors and bubble them up,
if would be nice if you used .context() from anyhow to preserve the
error messages. E.g.:


     let priv_user = pdm_config::priv_user().context("could not get privileged user")?;

>  
> +    proxmox_product_config::init(api_user.clone(), priv_user);
>      proxmox_access_control::init::init(
>          &pdm_api_types::AccessControlConfig,
>          pdm_buildcfg::configdir!("/access"),
> -    )
> -    .expect("failed to setup access control config");
> -
> +    )?;
>      proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO)
>          .stderr()
> -        .init()
> -        .expect("failed to set up logger");
> +        .init()?;
>  
> -    server::context::init().expect("could not set up server context");
> +    server::context::init()?;
>  
>      let cmd_def = CliCommandMap::new()
>          .insert("remote", remotes::cli())
> @@ -40,14 +36,29 @@ fn main() {
>          .insert("support-status", support_status::cli())
>          .insert("versions", CliCommand::new(&API_METHOD_GET_VERSIONS));
>  
> +    let args: Vec<String> = std::env::args().collect();
> +    let avoid_init = args.len() >= 2 && (args[1] == "bashcomplete" || args[1] == "printdoc");
> +
> +    if !avoid_init {
> +        let file_opts = CreateOptions::new().owner(api_user.uid).group(api_user.gid);
> +        proxmox_rest_server::init_worker_tasks(pdm_buildcfg::PDM_LOG_DIR_M!().into(), file_opts)?;
> +
> +        let mut command_sock = proxmox_daemon::command_socket::CommandSocket::new(api_user.gid);
> +        proxmox_rest_server::register_task_control_commands(&mut command_sock)?;
> +        command_sock.spawn(proxmox_rest_server::last_worker_future())?;
> +    }
> +
>      let mut rpcenv = CliEnvironment::new();
>      rpcenv.set_auth_id(Some("root@pam".into()));
>  
> -    run_cli_command(
> -        cmd_def,
> -        rpcenv,
> -        Some(|future| proxmox_async::runtime::main(future)),
> -    );
> +    run_async_cli_command(cmd_def, rpcenv).await;
> +
> +    Ok(())
> +}
> +
> +fn main() -> Result<(), Error> {
> +    //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to proxmox-sys and uncomment
> +    proxmox_async::runtime::main(run())
>  }
>  
>  #[api(



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


  reply	other threads:[~2026-01-27  9:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23 17:29 [pdm-devel] [PATCH datacenter-manager 0/3] fix #7179: expose ACME commands inside admin CLI Shan Shaji
2026-01-23 17:29 ` [pdm-devel] [PATCH datacenter-manager 1/3] cli: admin: make cli handling async Shan Shaji
2026-01-27  9:17   ` Lukas Wagner [this message]
2026-01-27 11:27     ` Shan Shaji
2026-01-23 17:29 ` [pdm-devel] [PATCH datacenter-manager 2/3] fix #7179: cli: admin: expose acme commands Shan Shaji
2026-01-27  9:17   ` Lukas Wagner
2026-01-27 11:14     ` Shan Shaji
2026-01-27 12:09       ` Lukas Wagner
2026-01-23 17:29 ` [pdm-devel] [PATCH datacenter-manager 3/3] chore: update proxmox-acme to version 1 Shan Shaji

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFZ9BS9TCBLM.3GTYR8FR24RZ6@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.shaji@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal