Jan Zeleny wrote:
Rob Crittenden<rcrit...@redhat.com>  wrote:
Jan Zelený wrote:
Rob Crittenden<rcrit...@redhat.com>   wrote:
Jan Zelený wrote:
Martin Kosek<mko...@redhat.com>    wrote:
On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote:
On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote:
Rob Crittenden<rcrit...@redhat.com>    wrote:
Jan Zelený wrote:
Martin Kosek<mko...@redhat.com>     wrote:
On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote:
Lookup based on --filter wasn't implemented at all. It did't
show until now, because of bug sitting on top of it which was
resulting in internal error. This patch fixes the bug and adds
the filtering functionality.

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

NACK

Did you build this patch on current master? Because in your
patch, you removed changes in permission-find from my previous
patch "017 ACI plugin supports prefixes". After your patch,
permission-find fails:

$ ipa permission-find
ipa: ERROR: 'aciprefix' is required

Martin

Sorry, I accidentaly mixed the code with a part of the older one.
Sending corrected patch.

Jan

I think the more stuff in baseldap.py:LDAPSearch() was there
because adding entries in a post_callback wasn't working. It only
let you reduce the number or modify what was already there IIRC.

  From what I know, lists should allow you to expand them without
  any

problems

(not sure how is the concept called in Python, Pavel told me about
it). Also I didn't encounter any problems with this approach (and
the post callback actually adds some entries), that's why I changed
it the way I did.

Jan

ACK

I think the concept of adding new items to list 'entries' is right.

Martin

Second-thought-NACK

After some thoughts about permissions and ACIs I think the ACI
filtering should be moved to ACI plugin - aci_find command. So that
it is available to other commands built over ACI plugin that would
need searching by filter.

A good place to move the filtering by 'filter' would be instead of the
following comment in aci.py:

# TODO: searching by: filter, subtree

Martin

Good catch. I'm sending another version of the patch in attachment.

Jan

This only does filter exact matches, is that adequate or should we
return any filter that has the query as a substring?

rob

I thought about that as well. If you think it is more appropriate, I'll
update the patch. But IMO this behavior is what users will expect.

Jan

Ok, I pushed this to master. Can you open a ticket to do substring
searches? I think it might be handy to have at some point, not enough of
a priority to hold the rest of this up.

rob

Sure, will do. As we discussed this with Jakub and Martin, this feature would
be handy not only here, but elsewhere as well. Hence it might be useful to
implement it in baseldap (if possible).

For LDAP-based entries this already happens, see ldap2.make_filter().

The permissions plugin does a lot of stuff difference since we do the search manually as opposed to over LDAP.

rob

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

Reply via email to