<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/17/2015 08:01 AM, Jan Cholasta
wrote:<br>
</div>
<blockquote cite="mid:5507D13E.7040107@redhat.com" 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; instead
<br>
import the module itself and use e.g.
`baseldap.LDAPObject`.
<br>
<br>
The stageuser help (docstring) is copied from
the user 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 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 plugin.
<br>
Don't do that. Either import things (e.g.
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 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), including the last change comment below it.
<br>
<br>
<br>
IMO baseuser should include superclasses for all the usual
commands (add, mod, del, show, find) and stageuser/deleteuser
commands should inherit from them.
<br>
<br>
<br>
You don't need to override class properties like
active_container_dn and takes_params on baseuser subclasses when
they have the same value as in baseuser.
<br>
<br>
<br>
Honza
<br>
<br>
</blockquote>
<font face="Times New Roman, Times, serif">Hello Honza,<br>
<br>
</font>
<blockquote>Thanks for the review. I did the modifications you
recommended within that attached patches<br>
<ul>
<li>Change version</li>
<li>create the baseuser_* plugins commands and use them in the
user/stageuser plugin commands</li>
<li>Do not redefine the class properties in the subclasses.</li>
</ul>
<p>Thanks<br>
thierry<br>
</p>
</blockquote>
</body>
</html>