[Freeipa-devel] 0165-0174 LDAP code refactoring (Part 5)

[Petr Viktorin]

On 02/27/2013 04:00 PM, Jan Cholasta wrote:
> On 31.1.2013 11:03, Petr Viktorin wrote:
>> And hee is another batch of patches. This one is about converting the
>> legacy IPAdmin and raw python-ldap calls to the new wrappers.
>>
>
> Patch 165:
>
> I have noticed two things that are not really related to your work, but
> here they are nonetheless:
>
> +        if self.admin_conn.get_entries(
> +                DN(api.env.container_ranges, self.suffix),
> +                ldap.SCOPE_ONELEVEL,
> +                "objectclass=ipaDomainIDRange"):
>
> Is that a valid filter?

I don't think it is, fixed.

> +        if self.admin_conn.get_entries("cn=accounts," + self.suffix,
> +                                       ldap.SCOPE_SUBTREE, id_filter):
>
> This doesn't seem right as well, why is the DN class not used here?

I made it a DN.

> Patch 167:
>
> -                conn.sasl_interactive_bind_s(None, sasl_auth)
> +                conn.do_sasl_gssapi_bind()
>
> sasl_auth is unused after this change, can you please remove it as well
> (and cb_info too)?

Removed.

> -            self.sasl_interactive_bind_s, timeout, None, SASL_AUTH)
> +            self.conn.sasl_interactive_bind_s, timeout, None, SASL_AUTH)
>
> Again, this is not related to your work, but can we please rename
> SASL_AUTH to SASL_GSSAPI?

OK, makes sense

> Patch 173:
>
> -        res = con.search_st(str(base),
> -                            ldap.SCOPE_SUBTREE,
> -                            filterstr=srcfilter,
> -                            attrlist=attrs,
> -                            timeout=10)
> +        res = con.get_entries(base, con.SCOPE_SUBTREE, srcfilter, attrs)
>
> I assume the timeout is there for a reason, can you please keep it?

Fixed, thanks

> Patch 174:
>
> -                    conn.modify_s(
> -                        def_dn,
> -                        [(ldap.MOD_REPLACE,
> -                        'originfilter',
> -                        disable_attr)]
> -                    )
> +                    entry['originfilter'] = [disable_attr]
>
> I think you forgot to call update_entry here.

Fixed, thanks again.


-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0165-02-Remove-search_s-and-search_ext_s-from-IPAdmin.patch
Type: text/x-patch
Size: 8992 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130227/9fe36a68/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0167-03-Remove-IPAdmin.sasl_interactive_bind_s.patch
Type: text/x-patch
Size: 4354 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130227/9fe36a68/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0173-03-Use-IPAdmin-rather-than-raw-python-ldap-in-ipactl.patch
Type: text/x-patch
Size: 5612 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130227/9fe36a68/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0174-04-Remove-some-uses-of-raw-python-ldap.patch
Type: text/x-patch
Size: 37190 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130227/9fe36a68/attachment-0003.bin>