Re: [pdm-devel] [PATCH datacenter-manager v2 09/12] api: subscription status: add support for view-filter parameter

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

On Wed Nov 5, 2025 at 3:28 PM CET, Dominik Csapak wrote:
> On 11/5/25 3:11 PM, Lukas Wagner wrote:
>> On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
>>>>    
>>>> -            let mut state = RemoteSubscriptionState::Unknown;
>>>> +            let state = if let Some(node_status) = &node_status {
>>>> +                if error.is_some() && view_filter_clone.is_some() {
>>>> +                    // Don't leak the existence of failed remotes, since we cannot apply
>>>> +                    // view-filters here.
>>>
>>> why not? we can check if the remote should be included, that does not
>>> requires any more info?
>>>
>>>
>> 
>> The problem is that we cannot always reliably tell if a remote is
>> included or not without checking the remote's resources. There is
>> ViewFilter::can_skip_remote - but since that one only checks
>> `include/exclude remote:...` rules, it might return false (meaning,
>> the remote must be considered) even if at the end the data from this
>> remote might be filtered out due to the other rules (and if no resources
>> remain, the remote as a whole would then be filtered out from the
>> response - similar to what I did in patch 6 to which you replied).
>> 
>> Example:
>> 
>> Consider only having `include tag:sometag` in the config. From this
>> alone, we cannot exclude any remotes from being queried a priori and
>> must do *all* filtering when we already have the data.
>> 
>> Similar thing here. If we cannot reach the remote, then we don't have a
>> list of nodes for this remote. Without the list of nodes, we cannot do
>> any checks with the node resource. This means if we do not filter out
>> the failed remote, we would leak its existence in a view which
>> potentially would not match on any of the node's resources.
>> 
>> I hope you could follow this explanation. Do you maybe have an idea on
>> how to fix this?
>> 
>> Only thing that comes to mind is making the include/exclude
>> remote:{remote} rules required, so that a view *always* needs to specify
>> which remotes to (not) consider. If we make these required, we probably
>> should make these a separate key in the config file again.
>> 
>
> ok i understand the issue, but instead of excluding all remotes that
> have errors, we could at least include those that we know should be
> included by the 'remote' filter?
>
> IMHO a remote is already a resource, so if i include some remote with
> it's name (or by type, etc.), even if it does not contain any sub
> resources, i should get back an (empty) remote
>
> does that make sense?
>
> i get we can't include/exclude based on other filters though, that
> must remain hidden as you mentioned, but for explicitly included
> remotes we should include them also with errors.
>

Yeah, I think this is a reasonable approach. I'll try this for v3.

Thanks for the feedback, Dominik!


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