[pve-devel] [PATCH common v3 1/1] PBSClient: file_restore_list: add timeout parameter

[Thomas Lamprecht]

Am 08/11/2022 um 12:20 schrieb Dominik Csapak:
> On 11/7/22 15:17, Thomas Lamprecht wrote:
>> subject is not wrong but worded rather confusingly, as of now it rather
>> implies that this adds a new parameter allowing callers to control the
>> timeout, but actually it sets the timeout hard-coded to 25s.
>>
>> Am 27/05/2022 um 10:22 schrieb Dominik Csapak:
>>> we always want the restore_list to use a timeout here. Set it to 25 seconds
>>
>> Such statements could be a bit more useful with some actual reasoning
>> (e.g., short sentence about ill effects of lacking timeout)
> 
> ok i thought the sentence below would be enough reasoning
> 

not really as it doesn't explains much for **why** headroom would be
required, if the clients gets the response cut short anyway at 30s
what benefits do we gain here, just another error message or otherwise
improved behavior? Why not just alarm($foo) on the call site.

main point is, it really doesn't hurts to have that relevant information
here too, not just in the pbs side of the commit..

>>
>>> so there is a little headroom between this and pveproxys 30s one.
>>
>> what if we'd add a call site outside the sync API response context
>> (e.g., task worker or CLI rpcenv)? could be an artificial limitation
>> in that case.
> 
> i followed your suggestion from the v1 version by hardcoding the options
> and you applied the pbs ones from v2 partially without
> complaining about this ;)

You mean
https://lists.proxmox.com/pipermail/pve-devel/2022-February/051664.html
? That was a comment about a horrid general "pass anything" interface
Wolfgang agreed too, not specific to the timeout param and it's implications,
that's why I asked here about if you thought about that (which you did not
replied at all...)

> 
> in any case, since i have to touch this again when doing the
> user dependent memory increase for the file restore,
> i'd rather use the other approach wolfang mentioned
> by having a %param hash with the 'timeout' (and
> dynamic memory) option.
> 
> i'd send these two things together in one (pve) series.
> is that ok for you?
> 

not sure, my question about what happens if I call this in CLI and if,
whatever does, should happen is not really answered.