On 03/07/2013 08:27 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/06/2013 09:52 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
[...]
On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
Assignment Plugin,cn=plugins,cn=config is added before the entry
itself.
I didn't test everything as I didn't get the access.
It shouldn't make a difference. What isn't working?
I get a CRITICAL message in the log:
add aci:
(targetattr=dnaNextRange || dnaNextValue ||
dnaMaxValue)(version 3.0;acl "permission:Modify DNA Range";allow (write)
groupdn = "ldap:///cn=Modify DNA
Range,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)
modifying entry "cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config"
2013-03-07T11:01:48Z DEBUG stderr=ldap_initialize(
ldap://vm-081.idm.lab.eng.brq.redhat.com:389/??base )
ldap_modify: No such object (32)
2013-03-07T11:01:48Z CRITICAL Failed to load replica-acis.ldif: Command
'/usr/bin/ldapmodify -v -f /tmp/tmpT55upM -H
ldap://vm-081.idm.lab.eng.brq.redhat.com:389 -x -D cn=Directory Manager
-y /tmp/tmplFeere' returned non-zero exit status 32
Gotcha. I moved where the replica acis are loaded.
Please attach the patch :)
[...]
+ failed = 0
+ for ent in entries:
This loops more than necessary and is somewhat hard to follow. Consider
using for-else here:
for ...:
...
if okay:
break
else:
raise error
I simplified things a bit but a for/else won't work here as we need to
check all ranges all the time. It is perfectly fine to not fit into a
range, as long as it fits into SOME range.
Well, that's how for's (not if's) else clause works -- it's executed
after all the looping's done if you didn't `break` out.
http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
Maybe I'm just used to it and it's too esoteric to the average reader,
though.
Thanks for the vote of confidence. Like I said, I wanted it to check all
the ranges. A for/else quits on the break, which I guess is really
probably ok assuming we trust that nothing else is going to stuff bad
ranges in. I can go ahead and make the change.
Your code also breaks out as soon as a good range is found.
Anyway this is a small nitpick; the loop works fine as it is.
[...]
Ok, I'll drop this since it doesn't affect things with the new LDAP
backend.
Please do, you left it in by mistake.
Yeah, there it is sitting unsquashed in my tree :-(
>
I also added one change related to the LDAP core changes. In the past if
you did not have a ticket it would prompt for DM password. This was
broken after the updates. I added an additional except in
test_connection().
This should also be fixed soon in ipaldap. Thanks for putting up with
the changes.
So should I drop this in my patch then? I don't really like having to
import ldap.
You can if you're fine with waiting until my patches are pushed.
Otherwise it's covered by https://fedorahosted.org/freeipa/ticket/3499
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
