On 06/18/2015 03:42 PM, David Kupka wrote:
Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a):
On 06/18/2015 12:52 PM, Jan Cholasta wrote:
Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a):
Dne 15.6.2015 v 17:29 thierry bordaz napsal(a):
On 06/15/2015 05:00 PM, Simo Sorce wrote:
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
good to
me and is definitely "alpha ready".

I found following issues but don't insist on fixing it right

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
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user

already exists
Hi David, Jan,

Thanks you so much for all those tests and feedback. I agree,
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
options for user-find command instead there is '--preserved'
Honza proposed adding virtual boolean attribute 'deleted' to
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
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
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
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.
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
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
unsure if we
agreed to finish it now or later.
Yes thanks
5) Twice deleting user with '--preserve' deletes him
$ 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
+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
one boolean parameter rather than two mutually exclusive flags in
I would like an opinion on this as well.

So the proposal is, e.g.,:

    ipa user del fbar --preserve
    ipa user del fbar --permanently
    ipa user del fbar --permanently=False
    ipa user del fbar --permanently=True
    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
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
user I would like the current options.

Given that Web UI or any other API client should not define CLI, I
keep the current options.

my 2c


Petr Vobornik

+1 --preserve is 100x better for a human than --permanently=False

I also prefere --preserve for usability of  'user del'.

In addition we have 'user find|show --preserved' to retrieve users
have been preserved. So it seems to me better that the action that
preserved the user uses the option '--preserve' rather

It's ridiculous that the CLI taints the RPC API and it should be fixed.

Also on a more nitpicky side, I think the flag should be called
--no-preserve rather than --permanently. There is plenty of commands
(rm, cp, ...) which have --no-preserve as opposite of --preserve.

The attached patch fixes both.

... and it also accidentaly changes the default behavior.

Updated patch attached.

ACK if others are ok with changing --permanently to --no-preserve.

Patch 446 fixed also issue #5, patch 446.1 doesn't fix it. Could be
fixed separately.

Attaching patch which addresses this API change in Web UI.

pvoborni's patch 0880 works for me, ACK. I also applied jcholast's patch
446.1 and did not encounter any issue.

pushed to master:
* 1d608251383e4842b89c941a76dbd13529558f42 User life cycle: change user-del flags to be CLI-specific * baca55c665b2bdfa5cb9a6ad88daeccef0500999 webui: adjust user deleter dialog to new api

Petr Vobornik

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to