On 05/21/2014 10:00 PM, Dmitri Pal wrote:
On 05/19/2014 10:45 AM, thierry bordaz wrote:
On 05/19/2014 04:44 PM, Jan Cholasta wrote:
On 19.5.2014 16:34, thierry bordaz wrote:
On 05/19/2014 04:22 PM, Jan Cholasta wrote:
On 19.5.2014 16:03, thierry bordaz wrote:
On 05/19/2014 03:54 PM, Jan Cholasta wrote:
On 19.5.2014 15:19, Petr Viktorin wrote:
Hello list,
Here's a conversation that started internally. I'm making it
public.

On 05/19/2014 01:00 PM, Martin Kosek wrote:
On 05/19/2014 12:46 PM, Petr Viktorin wrote:
On 05/19/2014 08:25 AM, Martin Kosek wrote:
On 05/19/2014 08:24 AM, Martin Kosek wrote:
On 05/16/2014 04:48 PM, thierry bordaz wrote:
Hello Martin,

     I am getting familiar with the freeipa CLI code and
started
     implemented '--to-stage' and '--from-stage'. This
really an
     impressive set of code :-)

Great! :-)

     I completed 'to-stage' and testing '--from-stage'.

     I have a question regarding the '--from-stage' syntax.
'uid'
is a
     mandatory argument to 'user-add' subcommand. In the
design the
     '--from-stage' option is described with:

         ipa user-add --from-stage=tuser

Note, the design is here:
http://www.freeipa.org/page/V4/User_Life-Cycle_Management

     But as 'uid' is mandatory the command should rather be

         ipa user-add tuser --from-stage=tuser

     In that case the option value for '--from-stage' is not
required and
     the command should be

         ipa user-add tuser --from-stage

     Is that ok if I implement the command like above or did I
miss
     something ?

     regards
     thierry

Hmm, no, I think you are right.  We can change --from-stage to
just
Bool
parameter. When it is true, it'd mean that get_dn or
pre-callback
should
retrieve the record from stage and use all it's attributes (and
add
standard
default attributes values on top of that).

Also CC-ing Petr Viktorin for reference.

This operation can't change the user's attributes, can it?
I.e., we
don't
support something like:
     ipa user-add tuser --from-stage --phone=123456789
--email=newem...@example.com
If this is the case, what's the reason for using user-add for
this?
Wouldn't it
be better to make this a separate command, say:
     ipa user-activate tuser
     ipa user-activate tuser --from-deleted
     ipa user-activate tuser --from-deleted --to-staged

+1, I would even go as far as having separate commands for staged
and
deleted users, e.g.:

    ipa user-unstage tuser
    ipa user-undelete tuser
    ipa user-undelete tuser --to-staged

A deleted entry has already been active so it contains already set
attributes while the pure staged entries are "almost" empty boxes.
But
from an administrator point of view, both staged/deleted entries are
inactive. What would be the advantages of two separated commands ?

You just said it yourself: activating/unstaging a user is quite
different from undeleting a user. Cramming multiple different
operations in a single command is bad design IMHO.

Ok I understand.
I believe that deleted entries and staged entries will be in the same
container (provisioning).

The design page mentions "cn=staged
users,cn=accounts,cn=provisioning,$SUFFIX" and "cn=deleted
users,cn=accounts,cn=provisioning,$SUFFIX", which are two different
containers.

Oppsss.. Sorry for the confusion :-[

So we may have at least those two possibilities:

  * ipa user-activate tuser [--from-staging|--from-delete]
  * ipa user-unstage tuser
    ipa user-undelete tuser


I like the idea of different verbs for different hives.
Something like:

Adding directly to stage via CLI: ipa user-stage
Removing from stage: user-unstage (user is gone)
Stage to Main -> activate; <- deactivate
Main to delete -> del; <-restore or undelete
Delete to stage -> I think we can use ipa user-stage command with
--deleted=user or similar

To be honest, I don't like this idea.
Too many names are confusing, if we can find a consistent option to cut the number of names down we should do it. IMO This is the worst part of Git: http://assets.osteele.com/images/2008/git-transport.png . We can do better.

Another good thing would be if options did not affect the applicability of other options (too much). For example in your proposal there'd be something like:
    ipa user-stage tuser --first=abc --last=xyz --phone=123 ......
    ipa user-stage --deleted=tuser  # <no attribute options allowed>
We should avoid this, if only for the reason that it makes the help text confusing.


My proposal would be that the move commands use the verb for the target and an option for the source, and add/mod use an option for the container:

1) adding a new user
(to active)   ipa user-add tuser ...
(to stage)    ipa user-add tuser --staged ...
(to deleted)  ipa user-add tuser --deleted ...  (*)

2) moving to main
(from stage)  ipa user-activate tuser  (**)
(from del)    ipa user-activate tuser --deleted

3) moving to deleted
(from active) ipa user-del tuser
(from stage)  ipa user-del tuser --staged

4) moving to stage
(from active) ipa user-stage tuser
(from del)    ipa user-stage tuser --deleted

5) modifying
(in active)   ipa user-mod tuser ...
(in stage)    ipa user-mod tuser --staged ...
(in del)      ipa user-mod tuser --deleted ...

Five commands (two of which are user-specific), plus two fairly consistent options.

If the delete container isn't configured, the --deleted option is illegal and `user-del` deletes permanently.


(*) may be useful in some situations?
(**) for full consistency this would be `ipa user-activate --staged`, but this is a good exception to make.


This is just an example. Pick the commands the way you see fit better
but IMO CLI should provide the whole cycle of moving entries around and
it should be very unambiguous what each means.

Please do not forget to update the design based on the results of this
discussion.











user-add command does a lot of additional processing besides just
taking the
values and writing them to LDAP. It fills the UID and GID, sets
the
non-filled
default attributes like Kerberos attributes, adds user as a
member of
ipausers
groups - all that stuff. The same procedures should be also
done with
the user
from stage. This is why I proposed to augment user-add.

If there is a better way, I am open to it.

That's not a very good reason to bring in all the CLI/API options,
most
importantly from the user's perspective. Also you'd have to write
extra
code to e.g. check the user didn't use the other options, and that
tends
to get messy quite fast.

The common processing should be split out into functions* that both
commands would call.
(Or methods of the `user` object, which may turn out to be more
practical.)










_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel




--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to