<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 03/19/2015 07:37 AM, Jan Cholasta
wrote:<br>
</div>
<blockquote cite="mid:550A6E97.9010103@redhat.com" type="cite">Dne
18.3.2015 v 19:39 thierry bordaz napsal(a):
<br>
<blockquote type="cite">On 03/17/2015 08:01 AM, Jan Cholasta
wrote:
<br>
<blockquote type="cite">Dne 16.3.2015 v 12:06 David Kupka
napsal(a):
<br>
<blockquote type="cite">On 03/06/2015 07:30 PM, thierry bordaz
wrote:
<br>
<blockquote type="cite">On 02/19/2015 04:19 PM, Martin Basti
wrote:
<br>
<blockquote type="cite">On 19/02/15 13:01, thierry bordaz
wrote:
<br>
<blockquote type="cite">On 02/04/2015 05:14 PM, Jan
Cholasta wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
Dne 4.2.2015 v 15:25 David Kupka napsal(a):
<br>
<blockquote type="cite">On 02/03/2015 11:50 AM,
thierry bordaz wrote:
<br>
<blockquote type="cite">On 09/17/2014 12:32 PM,
thierry bordaz wrote:
<br>
<blockquote type="cite">On 09/01/2014 01:08 PM,
Petr Viktorin wrote:
<br>
<blockquote type="cite">On 08/08/2014 03:54
PM, thierry bordaz wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
The attached patch is related to 'User
Life Cycle'
<br>
(<a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3813">https://fedorahosted.org/freeipa/ticket/3813</a>)
<br>
<br>
It creates a stageuser plugin with a first
function
<br>
stageuser-add.
<br>
Stage
<br>
user entries are provisioned under
'cn=staged
<br>
users,cn=accounts,cn=provisioning,SUFFIX'.
<br>
<br>
Thanks
<br>
thierry
<br>
</blockquote>
<br>
Avoid `from ipalib.plugins.baseldap import
*` in new code;
<br>
instead
<br>
import the module itself and use e.g.
`baseldap.LDAPObject`.
<br>
<br>
The stageuser help (docstring) is copied
from the user
<br>
plugin, and
<br>
discusses things like account lockout and
disabling users. It
<br>
should
<br>
rather explain what stageuser itself does.
(And I don't very
<br>
much
<br>
like the Note about the interface being
badly designed...)
<br>
Also decide if the docs should call it
"staged user" or "stage
<br>
user"
<br>
or "stageuser".
<br>
<br>
A lot of the code is copied and pasted over
from the users
<br>
plugin.
<br>
Don't do that. Either import things (e.g.
<br>
validate_nsaccountlock)
<br>
from the users plugin, or move the reused
code into a shared
<br>
module.
<br>
<br>
For the `user` object, since so much is the
same, it might be
<br>
best to
<br>
create a common base class for user and
stageuser; and similarly
<br>
for
<br>
the Command plugins.
<br>
<br>
The default permissions need different
names, and you don't need
<br>
another copy of the 'non_object' ones. Also,
run the makeaci
<br>
script.
<br>
<br>
</blockquote>
Hello,
<br>
<br>
This modified patch is mainly moving
common base class into a
<br>
new
<br>
plugin: accounts.py. user/stageuser plugin
inherits from
<br>
accounts.
<br>
It also creates a better description of
what are stage user,
<br>
how
<br>
to add a new stage user, updates ACI.txt
and separate
<br>
active/stage
<br>
user managed permissions.
<br>
<br>
thanks
<br>
thierry
<br>
<br>
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
</blockquote>
<br>
<br>
Thanks David for the reviews. Here the last
patches
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
<br>
</blockquote>
<br>
The freeipa-tbordaz-0002 patch had trailing
whitespaces on few
<br>
lines so
<br>
I'm attaching fixed version (and unchanged patch
<br>
freeipa-tbordaz-0003-3
<br>
to keep them together).
<br>
<br>
The ULC feature is still WIP but these patches
look good to me and
<br>
don't
<br>
break anything as far as I tested.
<br>
We should push them now to avoid further rebases.
Thierry can then
<br>
prepare other patches delivering the rest of ULC
functionality.
<br>
</blockquote>
<br>
Few comments from just reading the patches:
<br>
<br>
1) I would name the base class "baseuser", "account"
does not
<br>
necessarily mean user account.
<br>
<br>
2) This is very wrong:
<br>
<br>
-class user_add(LDAPCreate):
<br>
+class user_add(user, LDAPCreate):
<br>
<br>
You are creating a plugin which is both an object
and an command.
<br>
<br>
3) This is purely subjective, but I don't like the
name
<br>
"deleteuser", as it has a verb in it. We usually
don't do that and
<br>
IMHO we shouldn't do that.
<br>
<br>
Honza
<br>
<br>
</blockquote>
<br>
Thank you for the review. I am attaching the updates
patches
<br>
<br>
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
</blockquote>
Hello,
<br>
I'm getting errors during make rpms:
<br>
<br>
if [ "" != "yes" ]; then \
<br>
./makeapi --validate; \
<br>
./makeaci --validate; \
<br>
fi
<br>
<br>
/root/freeipa/ipalib/plugins/baseuser.py:641 command
"baseuser_add"
<br>
doc is not internationalized
<br>
/root/freeipa/ipalib/plugins/baseuser.py:653 command
"baseuser_find"
<br>
doc is not internationalized
<br>
/root/freeipa/ipalib/plugins/baseuser.py:647 command
"baseuser_mod"
<br>
doc is not internationalized
<br>
0 commands without doc, 3 commands whose doc is not i18n
<br>
Command baseuser_add in ipalib, not in API
<br>
Command baseuser_find in ipalib, not in API
<br>
Command baseuser_mod in ipalib, not in API
<br>
<br>
There are one or more new commands defined.
<br>
Update API.txt and increment the minor version in
VERSION.
<br>
<br>
There are one or more documentation problems.
<br>
You must fix these before preceeding
<br>
<br>
Issues probably caused by this:
<br>
1)
<br>
You should not use the register decorator, if this class
is just for
<br>
inheritance
<br>
@register()
<br>
class baseuser_add(LDAPCreate):
<br>
<br>
@register()
<br>
class baseuser_mod(LDAPUpdate):
<br>
<br>
@register()
<br>
class baseuser_find(LDAPSearch):
<br>
<br>
see dns.py plugin and "DNSZoneBase" and "dnszone"
classes
<br>
<br>
2)
<br>
there might be an issue with
<br>
@register()
<br>
class baseuser(LDAPObject):
<br>
<br>
the register decorator should not be there, I was warned
by Petr^3 to
<br>
not use permission in parent class. The same permission
should be
<br>
specified only in one place (for example user class),
(otherwise they
<br>
will be generated twice??) I don't know more details
about it.
<br>
<br>
--
<br>
Martin Basti
<br>
</blockquote>
<br>
Hello Martin, Jan,
<br>
<br>
Thanks for your review.
<br>
I changed the patch so that it does not register
baseuser_*. Also
<br>
increase the minor version because of new command.
<br>
Finally I moved the managed_permission definition out of
the parent
<br>
baseuser class.
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
Martin, could you please verify that the issues you
encountered are
<br>
fixed?
<br>
<br>
Thanks!
<br>
<br>
</blockquote>
<br>
You bumped wrong version variable:
<br>
<br>
-IPA_VERSION_MINOR=1
<br>
+IPA_VERSION_MINOR=2
<br>
<br>
It should have been IPA_API_VERSION_MINOR (at the bottom of
the file),
<br>
including the last change comment below it.
<br>
<br>
<br>
IMO baseuser should include superclasses for all the usual
commands
<br>
(add, mod, del, show, find) and stageuser/deleteuser commands
should
<br>
inherit from them.
<br>
<br>
<br>
You don't need to override class properties like
active_container_dn
<br>
and takes_params on baseuser subclasses when they have the
same value
<br>
as in baseuser.
<br>
<br>
<br>
Honza
<br>
<br>
</blockquote>
Hello Honza,
<br>
<br>
Thanks for the review. I did the modifications you
recommended
<br>
within that attached patches
<br>
<br>
* Change version
<br>
</blockquote>
<br>
Please also update the comment below (e.g. "# Last change: tbordaz
- Add stageuser_add command")
<br>
<br>
<blockquote type="cite"> * create the baseuser_* plugins
commands and use them in the
<br>
user/stageuser plugin commands
<br>
* Do not redefine the class properties in the subclasses.
<br>
</blockquote>
<br>
There are still some in baseuser command classes:
<br>
<br>
+class baseuser_add(LDAPCreate):
<br>
+ """
<br>
+ Prototype command plugin to be implemented by real plugin
<br>
+ """
<br>
+ active_container_dn = api.env.container_user
<br>
+ has_output_params = LDAPCreate.has_output_params
<br>
<br>
You don't need to set active_container_dn here, you only need to
set it in baseuser. Then in stageuser_add and other subclasses you
use "self.obj.active_container_dn" instead of
"self.active_container_dn".
<br>
<br>
You also don't need to override has_output_params if you are not
changing its value - you are inheriting from LDAPCreate, so
baseuser_add.has_output_params implicitly has the same value as
LDAPCreate.has_output_params.
<br>
<br>
<blockquote type="cite">
<br>
Thanks
<br>
thierry
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
Hello Honza,<br>
<br>
<blockquote>Thanks for your patience .. <span class="moz-smiley-s1"><span>
:-) </span></span><br>
I understand my mistake. Just a question, in a plugin command
(user_add), is 'self.obj' referring to the plugin object (like
'user') ?<br>
<br>
updated patches (with the appropriate naming and patch
versioning).<br>
<br>
thanks<br>
theirry<br>
</blockquote>
</body>
</html>