Re: [pdm-devel] [PATCH datacenter-manager 2/3] fix #7179: cli: admin: expose acme commands

["Lukas Wagner" <l.wagner@proxmox.com>]

On Tue Jan 27, 2026 at 6:33 PM CET, Shan Shaji wrote:
> On Tue Jan 27, 2026 at 1:09 PM CET, Lukas Wagner wrote:
>> On Tue Jan 27, 2026 at 12:14 PM CET, Shan Shaji wrote:
>>>>> @@ -21,13 +22,17 @@ async fn run() -> Result<(), Error> {
>>>>>          &pdm_api_types::AccessControlConfig,
>>>>>          pdm_buildcfg::configdir!("/access"),
>>>>>      )?;
>>>>> +    proxmox_acme_api::init(pdm_buildcfg::configdir!("/acme"), false)?;
>>>>> +
>>>>>      proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO)
>>>>> +        .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.
>>>
>>> or could we keep the `stderr_on_no_workertask()` and remove the
>>> `stderr`?
>>
>> No, then you would not get any log output from any code that runs within
>> the worker, that would not be ideal.
>>
>> Maybe check out the defintion of these builder methods in
>> proxmox-log/src/builder.rs, I think the doc comments makes the behavior
>> a bit more clear.
>>
>> Since this is a CLI tool, I would say that every log message that is
>> produced in the code should go to stderr in any case, as this might
>> contain important information for interactive or scripted use cases - so
>> `.stderr()`.
>
> Hey lukas, thank you so much for the detailed explanation. 
> Makes sense to register both the tasklog_pbs() and stderr() layers.  
>
>> As a bonus I think it's valuable to have logs of the worker tasks inside
>> the task logs as well, hence the `.tasklog_pbs()` (maybe we should
>> reconsider the name, this is not really PBS specific any more).
>
> then should i rename the function to `tasklog` and update all call sites
> in both PDM and PBS?

I think yes, but definitely talk to Gabriel first. Also I'd do this in a
separate patch series, since it's mostly a cosmetic issue.

If you do this, consider leaving the `tasklog_pbs` method in place for
backwards compatibility and just add a 
  #[deprecated(note = "use ... instead")]
so that there is just a warning when compiling, not a hard error.


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