Ondrej Hamada wrote:
On 02/28/2012 09:57 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
On 02/27/2012 03:22 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
When adding or modifying permission with both type and attributes
specified, check whether the attributes are allowed for specified
type.
In case of disallowed attributes the InvalidSyntax error is raised.

New tests were also added to the unit-tests.

https://fedorahosted.org/freeipa/ticket/2293

https://www.redhat.com/mailman/listinfo/freeipa-devel

NACK. You should use obj.object_class_config to determine if the
default list of objectclasses comes from LDAP.

I think that may be it, otherwise the patch reads ok.

I'm very glad to see unit tests!

rob
Corrected


Sorry, found a couple of more things I should have found the first
review.

Please use the dn module to construct dn_ipaconfig. Or you can also
get the DN on-the-fly since the config object using get_dn().

Probably just as safe to call: if obj.object_class_config: ... rather
than hasattr. I suppose its just a style thing.
Done.

I wonder if ObjectclassViolation is a better exception. SyntaxError
means the data type is wrong, not that it isn't allowed.
I agree that it makes more sense and I've updated the patch that way,
but the documentation says: "permission operation fails with schema
syntax errors" - maybe we should also update the documentation.

rob





I'll open a ticket on clarifying SyntaxError.

ACK, pushed to master and ipa-2-2

rob

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

Reply via email to