On 22.03.22 16:23, Dominik Csapak wrote: > On 3/22/22 14:44, Thomas Lamprecht wrote: >> On 22.03.22 07:11, Thomas Lamprecht wrote: >>> On 04.02.22 15:24, Dominik Csapak wrote: >>>> this deprecates the 'full' sync option and replaces it with >>>> a 'mode' option, where we add a third one that updates >>>> the current users (while retaining their custom set attributes not >>>> exisiting in the source) and removing users that don't exist anymore >>>> in the source >>>> >>> I'm not yet 100% sure about the specific mode names, as sync normally means >>> 100% sync, I'll see if I find some other tool (rsync?) with similar option >>> naming >>> problems. Independent from the specific names, this really needs a docs >>> patch, >>> ideally with a table listing the modi as rows and having the various "user >>> added", >>> "user removed", "properties added/updated", "properties removed" as >>> columns, for a >>> better understanding of the effects.. >>> >> A thought (train): what we decide with this isn't what gets added/updated, >> that's >> always the same, only what gets removed if vanished on the source, so maybe: >> >> remove-vanished: < none | user | user-and-properties > >> >> Or if we can actually also remove either user *or* group then: >> s/user/entity/ ? >> >> ps. the web interface should probably do a s/Purge/Purge ACLs/ too; or with >> that >> in mind we could actually drop that do and have: >> >> remove-vanished: < none | user | user-and-properties | >> user-and-properties-and-acl > >> >> And with that, we could go the separate semicolon-endcoded-flag-list like we >> do for >> some CT features (or mount options) IIRC: >> >> remove-vanished: [<user>];[<properties>];[acls] >> >> I.e., those three flags would replace your new mode + purge like: >> >> +--------+--------+---------------------+ >> | Mode | Purge | -> removed-vanished | >> +--------+--------+---------------------+ >> | update | 0 | "" (none) | >> | sync | 0 | user | >> | full | 0 | user;properties | >> | update | 1 | acl | >> | sync | 1 | acl;user | >> | full | 1 | acl;user;properties | >> +--------+--------+---------------------+ >> >> The selector for them could be either three check boxes on one line (similar >> to the >> privilege level radio buttons from CT restore) or even a full blown combobox >> with all >> the options spelled out. >> >> It's only slightly weird for acl, as there the "remove-vanished" somewhat >> implies that >> we import acl's in the first place, if we really don't want that we could >> keep >> "Purge ACLs" as separate option that is only enabled if "remove-vanished" >> "user" flag >> is set, put IMO not _that_ of a big problem to understand compared to the >> status quo. >> >> Does (any of) this make sense to you? > > yes this sounds sensible, but i agree about the possibly confusing > 'remove-vanished' > implication for acls. Maybe 'remove-on-vanish' ?
sounds the same to me semantically, so see no improvement there. > this would (semantically) decouple the 'vanished' thing from the 'removed' > thing, > at least a little bit. IMO purely subjective and if a real grammar/semantic connection would be there that I just miss (always a possibility) it'd be to subtle. I think that the confusion potential overall would get quite a bit reduced that getting this slightly confusing one newly is still a net benefit and can be easily defused with a short docs note. > in either case the docs would have to be updated anyway (as you already said) > > aside from that, i think line 4 in your table is not really practical, > since it would remove the acls but leave the users ? The user cannot do anything anymore (like auto-disable) but you still have a reference to it and all its configured fields + TFA, either to re-enable it later or to check contact info if one would investigate a specific task, so IMO its still practical for setups that want to auto-disable but not auto-remove. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
