Martin Kosek wrote:
On Fri, 2011-04-01 at 13:51 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
Target branches: master, ipa-2-0
---
Most of the pwpolicy_* commands do include cospriority in the result
and potentially in the attribute rights (--all --rights). Especially
when --raw output is requested. This patch fixes it for all
pwpolicy commands.
https://fedorahosted.org/freeipa/ticket/1103
nack. I see a couple of problems.
You should test for rights before doing the cosentry_show(). If rights
is False then we won't add the data whatever it is so it is more
efficient to exit earlier.
We have to call cosentry_show every time (except for the case when we
pull data for the global policy) because we read cospriority attribute.
But the function was indeed not efficient (it called cosentry_show
twice), I rewrote it.
Same with pwpolicy_name == global_policy_name. I think you should drop
the try/except and make it:
if not rights or pwpolicy_name == global_policy_name:
return
...
It should never be the case that the cosentry is not found so I'd let it
fail if that does occur.
Fixed.
I think that keys[-1] can be None so be aware.
Fixed.
You hardcode rights == False in pwpolicy_find(), a good thing. I think
you should add make it explict rights=False and add a comment explaining
that you can't get accessrights with a find.
Fixed.
Fixed patch attached.
Martin
Looks great, ack.
rob
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
