On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: > On 06/09/2015 02:02 PM, Jan Cholasta wrote: > > Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): > >> Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): > >>> On 05/15/2015 04:44 PM, David Kupka wrote: > >>>> Hello Thierry, > >>>> thanks for the patch set. Overall functionality of ULC feature looks > >>>> good to > >>>> me and is definitely "alpha ready". > >>>> > >>>> I found following issues but don't insist on fixing it right now: > >>>> > >>>> 1) When stageuser-activate fails due to already existent > >>>> active/deleted user. > >>>> DN is show instead of user name that's used in other commands > >>>> (user-add, > >>>> stageuser-add). > >>>> $ ipa user-add tuser --first Test --last User > >>>> $ ipa stageuser-add tuser --first Test --last User > >>>> $ ipa stageuser-activate tuser > >>>> ipa: ERROR: Active user > >>>> uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com > >>>> > >>>> > >>>> > >>>> already exists > >>> > >>> Hi David, Jan, > >>> > >>> Thanks you so much for all those tests and feedback. I agree, some minor > >>> bugs can be fixed separatly from this main patches. > >>> > >>> You are right, It should return the user ID not the DN. > >>> > >>>> > >>>> 2) According to the design there should be '--only-delete' and > >>>> '--also-delete' > >>>> options for user-find command instead there is '--preserved' option. > >>>> Honza proposed adding virtual boolean attribute 'deleted' to user > >>>> entry and > >>>> filter on it. > >>>> The 'deleted' attribute would be useful also in user-show where is no > >>>> way to > >>>> tell if the displayed user is active or deleted. (Except running with > >>>> --all > >>>> and looking on the dn). > >>> > >>> Yes a bit late to resynch the design. > >>> The final option is 'preserved' for user-find and 'preserve' for > >>> user-del. '--only-delete' or 'also-delete' are old name that I need to > >>> replace in the design. > >>> > >>> About the 'deleted' attribute, do you think adding a DS cos virtual > >>> attribute ? > >> > >> See the attached patch. > > > > Can someone please review the patch? > > > >> > >>> > >>>> > >>>> 3) uidNumber and gidNumber can't be set back to '-1' once set to other > >>>> value. > >>>> This would be useful when admin changes its mind and want IPA to > >>>> assign them. > >>>> IIUC, there should be no validation in cn=staged user container. All > >>>> validation should be done during stageuser-activate. > >>> > >>> Yes that comes from user plugin that enforce the number to be >0. > >>> That is a good point giving the ability to reset uidNumber/gidNumber. > >>> I will check if it is possible, how (give a value or an option to > >>> reset), and also if it would not create other issue. > >>>> > >>>> 4) Support for deleted -> stage workflow is still missing. But I'm > >>>> unsure if we > >>>> agreed to finish it now or later. > >>> > >>> Yes thanks > >>>> > >>>> 5) Twice deleting user with '--preserve' deletes him permanently. > >>>> $ ipa user-add tuser --first Test --last User > >>>> $ ipa user-del tuser --preserve > >>>> $ ipa user-del tuser --preserve > >>>> $ ipa user-find --preserved > >>>> ------------------------ > >>>> 0 (delete) users matched > >>>> ------------------------ > >>>> ---------------------------- > >>>> Number of entries returned 0 > >>>> ---------------------------- > >>> > >>> Deleting a deleted (preserved) entry, should permanently remove the > >>> entry. > > +1, but no-op if default behavior is "preserve" > > >>> Now if the second time the preserve option is present, it makes sense to > >>> not delete it. > > +1, should be no-op > > >> > >> BTW: I might be stating the obvious here, but it would be better to use > >> one boolean parameter rather than two mutually exclusive flags in > >> user-del. > > > > I would like an opinion on this as well. > > > > So the proposal is, e.g.,: > > Replace: > ipa user del fbar --preserve > ipa user del fbar --permanently > with: > ipa user del fbar --permanently=False > ipa user del fbar --permanently=True > and > ipa user del fbar > uses the default behavior(permanently atm.) > > I don't think there is a big difference. A boolean is easier for > scripting. 2 options are more descriptive for humans. With a single > boolean, I would be afraid that omitting it would imply False to some > users which is not always the same as "the default behavior" [1]. > > With Web UI developer hat I would vote for single boolean but as a CLI > user I would like the current options. > > Given that Web UI or any other API client should not define CLI, I would > keep the current options. > > my 2c > > [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User > -- > Petr Vobornik >
+1 --preserve is 100x better for a human than --permanently=False Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code