From: "Shan Shaji" <s.shaji@proxmox.com>
To: "Gabriel Goller" <g.goller@proxmox.com>,
"Lukas Wagner" <l.wagner@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager 2/3] fix #7179: cli: admin: expose acme commands
Date: Wed, 28 Jan 2026 10:35:25 +0100 [thread overview]
Message-ID: <DG04C71S8SEI.38MLTXHVLCIU@proxmox.com> (raw)
In-Reply-To: <st4ntre2rdsmqn2cnql6pwii6wuwtwsbftx6eexrpaekajtrbq@d7erkvogdcw3>
Thanks guys! I will make the changes accordingly.
On Wed Jan 28, 2026 at 10:26 AM CET, Gabriel Goller wrote:
> [...]
>> [...]
[...]
>> > + .stderr_on_no_workertask()
>> > .stderr()
>> > .init()?;
>>
>> Doing this actually prints all messages logged outside a workertask
>> *twice*, since this adds two subscribers that will print to stdout.
>>
>> I think it would make more sense to:
>>
>> proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO)
>> .tasklog_pbs()
>> .stderr()
>> .init()?;
>>
>> ... which should print *all* messages to stderr and messages from within
>> a task log will also be stored in the task log.
>>
>> Maybe also ask Gabriel about these, he did a lot of work on logging.
>>
>> @Gabriel, would my proposal make sense here?
>
> Yep, looks good!
>
> We can also deprecate `tasklog_pbs` in favor of a more generic `tasklog`
> function. This change is ok because pve's tasklog implementation is
> quite different from pbs, so we won't use this for pve workers anytime
> soon.
>
>> >
>> > server::context::init()?;
>> >
>> > let cmd_def = CliCommandMap::new()
>> > + .insert("acme", acme::acme_mgmt_cli())
>> > .insert("remote", remotes::cli())
>> > .insert(
>> > "report",
>>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
next prev parent reply other threads:[~2026-01-28 9:35 UTC|newest]
Thread overview: 14+ 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
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-27 17:33 ` Shan Shaji
2026-01-28 8:19 ` Lukas Wagner
2026-01-28 9:26 ` Gabriel Goller
2026-01-28 9:35 ` Shan Shaji [this message]
2026-02-03 17:55 ` Shan Shaji
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=DG04C71S8SEI.38MLTXHVLCIU@proxmox.com \
--to=s.shaji@proxmox.com \
--cc=g.goller@proxmox.com \
--cc=l.wagner@proxmox.com \
--cc=pdm-devel@lists.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.