From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 7B2991FF13E for ; Fri, 06 Feb 2026 13:56:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 023521E61; Fri, 6 Feb 2026 13:57:16 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 06 Feb 2026 13:56:40 +0100 Message-Id: Subject: Re: [PATCH datacenter-manager v2 1/4] cli: admin: make cli handling async From: "Shan Shaji" To: "Lukas Wagner" , X-Mailer: aerc 0.20.0 References: <20260203175101.457724-1-s.shaji@proxmox.com> <20260203175101.457724-2-s.shaji@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770382522004 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.110 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: INMHGSDVMGHHPEQPH546D4HE577ABEIF X-Message-ID-Hash: INMHGSDVMGHHPEQPH546D4HE577ABEIF X-MailFrom: s.shaji@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi Lukas, thank you so much for the nice feedbacks. I will apply some of the changes in my v3 and the improvements that you mentioned in a follow up series.=20 some comments inline.=20 On Thu Feb 5, 2026 at 3:25 PM CET, Lukas Wagner wrote: > Hi Shan, > [...] >> ) >> - .expect("failed to setup access control config"); >> - >> + .context("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"); >> + .context("failed to set-up logger")?; > > The string here changed, the original version seems to be more correct > to me I will change it back to "set up". Thank You! >> =20 >> - server::context::init().expect("could not set up server context"); >> + server::context::init().context("could not set-up server context")?= ; >> =20 >> let cmd_def =3D CliCommandMap::new() >> .insert("remote", remotes::cli()) >> @@ -40,14 +38,33 @@ fn main() { >> .insert("support-status", support_status::cli()) >> .insert("versions", CliCommand::new(&API_METHOD_GET_VERSIONS)); >> =20 >> + let args: Vec =3D std::env::args().collect(); >> + let avoid_init =3D args.len() >=3D 2 && (args[1] =3D=3D "bashcomple= te" || args[1] =3D=3D "printdoc"); > > For what it's worth, this could be > > let avoid_init =3D matches!( > args.get(1).map(String::as_str), > Some("bashcomplete") | Some("printdoc") > ); > or > > let avoid_init =3D args > .get(1) > .is_some_and(|c| c =3D=3D "bashcomplete" || c =3D=3D "printdoc"); > > It's not really shorter, but I think they would be a bit more idiomatic. > > The original version relies on the short-circuit behavior of the && > operator to avoid the panic when accessing the array element via the > index operator, which is of course correct, but I think this can be > written a bit nicer. > > no hard feelings, so just pick whichever version you like most. I would go with the `matches!` macro. Thank you! >> + >> + if !avoid_init { >> + let file_opts =3D CreateOptions::new().owner(api_user.uid).grou= p(api_user.gid); >> + proxmox_rest_server::init_worker_tasks(pdm_buildcfg::PDM_LOG_DI= R_M!().into(), file_opts) >> + .context("failed to initialize worker tasks")?; >> + >> + let mut command_sock =3D proxmox_daemon::command_socket::Comman= dSocket::new(api_user.gid); >> + proxmox_rest_server::register_task_control_commands(&mut comman= d_sock) >> + .context("failed to register task control commands")?; >> + command_sock >> + .spawn(proxmox_rest_server::last_worker_future()) >> + .context("failed to activate the socket")?; >> + } >> + >> let mut rpcenv =3D CliEnvironment::new(); >> rpcenv.set_auth_id(Some("root@pam".into())); >> =20 >> - 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 pr= oxmox-sys and uncomment >> + proxmox_async::runtime::main(run()) >> } >> =20 >> #[api(