On 09/26/2013 02:22 PM, Jan Cholasta wrote:
On 24.9.2013 15:35, Jan Cholasta wrote:
On 27.2.2013 16:31, Jan Cholasta wrote:
Hi,

these patches add the ability to access and manipulate raw attribute
values as they are returned from python-ldap to the LDAPEntry class.
This is useful for comparing entries, computing modlists for the modify
operation, deleting values that don't match the syntax of an attribute,
etc., because we don't need to care what syntax does a particular
attribute use, or what Python type is used for it in the framework (raw
values are always list of str). It should also make interaction with
non-389 DS LDAP servers easier in the framework.

(It might be too late for this kind of changes to get into 3.2 now, I'm
posting these patches mainly so that you are aware that they exist.)

Honza


This is now planned for 3.4:
<https://fedorahosted.org/freeipa/ticket/3521>

I fixed some issues in these patches and refined the API. Updated
patches attached.

Also added a patch to use raw values when adding new entries and a patch
which refines LDAPEntry.single_value, so that it is consistent with the
rest of the changes introduced in the patches.

Patch 110 will probably be dropped in favor of Petr Viktorin's schema
update patches, but I included it anyway.

Incidentally, this also fixes
<https://fedorahosted.org/freeipa/ticket/3927> and possibly also
<https://fedorahosted.org/freeipa/ticket/2131>.

Honza


Noticed a couple more issues and fixed them. Updated patches attached.

Honza

Thanks for the patches!


106. Make LDAPEntry a wrapper around dict rather than a dict subclass.

ipapython/ipaldap.py:847:
 warning: 1 line adds whitespace errors.

         if isinstance(_obj, LDAPEntry):
+            data = dict(_obj._data)
             orig = _obj._orig

Is this necessary? `self.update(_obj)` is done later.


     def __contains__(self, name):
-        return self._names.has_key(self._attr_name(name))
+        return self._names.has_key(name)

has_key() is deprecated for dict, it would make sense to prefer `name in self._names` for CIDict too.

+    def __eq__(self, other):
+        if not isinstance(other, LDAPEntry):
+            return NotImplemented
+        if self._dn != other._dn:
+            return False
+        return super(LDAPEntry, self).__eq__(other)

I don't think equality checking makes sense on a LDAPEntry, where you might have different capitalizations/variants of attribute names, different _orig, or a different set of attributes loaded on the same entry. It's not obvious which of those differences should make the entries inequal.
I'd just base it on identity (`self is other`).

     def __iter__(self):
         yield self._dn
         yield self

This makes the whole thing sort of hackish -- we need to reimplement everything in MutableMapping that uses iter() internally :(
Hopefully we can get rid of it soon.
I'd welcome FIXME comments on whatever is reimplemented for this reason.


107. Introduce IPASimpleLDAPObject.decode method for decoding LDAP values.

Can you put in a docstring?



108. Always use lists for values in LDAPEntry internally.

@@ -698,6 +701,7 @@ class LDAPEntry(collections.MutableMapping):

         result._names = deepcopy(self._names)
         result._data = deepcopy(self._data)
+        result._not_list = deepcopy(self._not_list)
         if self._orig is not self:
             result._orig = self._orig.clone()

It's better to use set() than deepcopy() for a set of strings.


109. Decode and encode attribute values in LDAPEntry on demand.

The syncing looks rather over-engineered to me.
Did you consider a custom MutableSequence for the values?
I think it would be much cleaner in the end than merging two sets of changes together.

I think it would also help (in the future?) to make the value lists more set-like, since the order doesn't matter.

--
PetrĀ³


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

Reply via email to