On Tue, 2011-10-11 at 13:16 +0300, Alexander Bokovoy wrote: > On Tue, 11 Oct 2011, Martin Kosek wrote: > > On Tue, 2011-10-11 at 12:01 +0300, Alexander Bokovoy wrote: > > > On Tue, 11 Oct 2011, Martin Kosek wrote: > > > > @@ -212,6 +216,24 @@ class config_mod(LDAPUpdate): > > > > raise errors.ValidationError( > > > > name=k, error='attribute "%s" not allowed' > > > > % a > > > > ) > > > Could you please also (in a separate patch) fix the above and others > > > by adding translations? Other exception messages in > > > plugins/config.py are designed to be used for user interactions but > > > this one is not localized. > > > > Patch based on 142 for config plugin i18n fix attached. I created a > ACK. > > > ticket to fix all of these issues in 3.0 branch: > > > > https://fedorahosted.org/freeipa/ticket/1953 > Thanks! > > > > > > > > > + > > > > + for (attr, obj) in (('ipauserobjectclasses', 'user'), > > > > + ('ipagroupobjectclasses', 'group')): > > > > + if attr in entry_attrs: > > > > + objectclasses = entry_attrs[attr] + > > > > self.api.Object[obj].possible_objectclasses > > > would it make sense to do sort(set(objectclasses)) to get unique list > > > before using it further? Just a thought. get_allowed_attributes() will > > > go to LDAP's schema to consult about the attributes and it seems to me > > > we'd better not to do this multiple times for the same. > > > > I added a list(set()) to remove duplicates, I don't think it is > > necessary to sort it. > Yes, this is fine. > > > > > + new_allowed_attrs = > > > > ldap.get_allowed_attributes(objectclasses, > > > > + raise_on_unknown=True) > > > > + checked_attrs = self.api.Object[obj].default_attributes > > > > + if self.api.Object[obj].uuid_attribute: > > > > + > > > > checked_attrs.append(self.api.Object[obj].uuid_attribute) > > > > + for obj_attr in > > > > self.api.Object[obj].default_attributes: > > > Shouldn't this be checked_attrs? > > > > > > > Correct. The previous version worked because .append modified the > > original list. Fixed. > Hm.. > + checked_attrs = self.api.Object[obj].default_attributes > doesn't change anything -- you still get a reference to > default_attributes and through > checked_attrs += .... > you'd modify the original one. Wouldn't the following be more correct: > + checked_attrs = copy(self.api.Object[obj].default_attributes) > ?
This was done on purpose. When you combine 2 lists in Python using + operator, a new list is created without modifying the old one. Check the following example: >>> a = [1,2,3] >>> b = [4] >>> c = a+b >>> print c [1, 2, 3, 4] >>> print a [1, 2, 3] >>> print b [4] >>> c.append(5) >>> print c [1, 2, 3, 4, 5] >>> print a [1, 2, 3] >>> print b [4] Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel