From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Philipp Hufnagl <p.hufnagl@proxmox.com>,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>,
Lukas Wagner <l.wagner@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/3] fix #4315: jobs: modify GroupFilter so include/exclude is tracked
Date: Wed, 25 Oct 2023 17:45:11 +0200 [thread overview]
Message-ID: <1b0eec17-d519-4d80-89f6-e08b9738fcd8@proxmox.com> (raw)
In-Reply-To: <b42c40b6-b8f9-4269-9440-d527fb940b97@proxmox.com>
Am 25/10/2023 um 17:07 schrieb Philipp Hufnagl:
>
>
> On 10/25/23 15:33, Thomas Lamprecht wrote:
>> Am 24/10/2023 um 16:32 schrieb Philipp Hufnagl:
>>>
>>>
>>> On 10/24/23 12:43, Lukas Wagner wrote:
>>>
>>>> Do you have any examples in mind that would be more difficult to
>>>> represent?
>>>
>>> I would like to include all vms from 10 to to 30, but not 17,18 and 20.
>>>
>>
>> How is that more difficult?
>>
>> IMO Lukas proposal seems reasonable, a deterministic remove matches from
>> excludes fromm all matches from includes seems easier to understand,
>> from top of my head.
>
> You would have to do something like
>
> include 10-30
> exclude 17
> exclude 18
> exclude 20
>
> instead of
>
> include 10-30
> exclude 17-20
> include 19
>
> While the first is easier to understand, the 2nd one allows (in my
> opinion) to build a cleaner solution for complex filtering.
Easier to understand is *much* cleaner though, any admin that changes
a simple include/exclude filter manually, e.g., reversing the order,
has no idea that the end result is completely different.
So if one would want to have such complex filtering, the order would
need to be explicitly encoded, and we always could extend a simpler
solution to a more complex one, while vice versa would be a breaking
change. But here I doubt I want to ever do that, most of the time one
has simple filter needs, servicing complex ones is error-prone not
only for the implementation but for the admin too, while only help
theoretical use cases.
Note that setups can already use namespaces for basic grouping,
and then simple exclude a few, or include a few, filters are enough
most of the time – having both, an include- and exclude- filter at the
same time is IMO already borderline overkill..
>
> However both are in my opinion fine solutions. I you think having 2
> lists is a better solution I can implement that as well.
As mentioned off-list, the actual implementation wouldn't be really
that different in either implementation.
Grouping or splitting exclusions and inclusions in the UI would
naturally be good then to better convey the basic principles.
next prev parent reply other threads:[~2023-10-25 15:45 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-23 15:42 [pbs-devel] [PATCH proxmox-backup 0/3] fix #4315: datastore: Exclude entries from sync Philipp Hufnagl
2023-10-23 15:43 ` [pbs-devel] [PATCH proxmox-backup 1/3] fix #4315: jobs: modify GroupFilter so include/exclude is tracked Philipp Hufnagl
2023-10-24 9:18 ` Lukas Wagner
2023-10-24 9:54 ` Philipp Hufnagl
2023-10-24 10:43 ` Lukas Wagner
2023-10-24 14:32 ` Philipp Hufnagl
2023-10-25 13:33 ` Thomas Lamprecht
2023-10-25 15:07 ` Philipp Hufnagl
2023-10-25 15:45 ` Thomas Lamprecht [this message]
2023-11-07 7:43 ` Wolfgang Bumiller
2023-11-07 7:55 ` Thomas Lamprecht
2023-11-07 8:26 ` Wolfgang Bumiller
2023-11-07 9:01 ` Dominik Csapak
2023-11-07 11:10 ` Thomas Lamprecht
2023-11-07 11:07 ` Thomas Lamprecht
2023-10-23 15:43 ` [pbs-devel] [PATCH proxmox-backup 2/3] ui: Show if Filter includes or excludes Philipp Hufnagl
2023-10-24 12:20 ` Lukas Wagner
2023-10-24 12:27 ` Lukas Wagner
2023-10-24 12:36 ` Philipp Hufnagl
2023-10-24 14:09 ` Philipp Hufnagl
2023-10-24 14:12 ` Lukas Wagner
2023-10-27 9:29 ` Thomas Lamprecht
2023-10-23 15:43 ` [pbs-devel] [PATCH proxmox-backup 3/3] docs: document new include/exclude paramenter Philipp Hufnagl
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=1b0eec17-d519-4d80-89f6-e08b9738fcd8@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=l.wagner@proxmox.com \
--cc=p.hufnagl@proxmox.com \
--cc=pbs-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox