[Freeipa-devel] [PATCH] Add missing _sasl_auth variable and fix some minor bugs, all in ldap2.

[Jason Gerard DeRose]

On Tue, 2009-04-21 at 16:40 +0200, Pavel Zuna wrote:
> Rob Crittenden wrote:
> > Pavel Zuna wrote:
> >> ldap2 currently isn't functional without this patch.
> >>
> >> Pavel
> > 
> > This patch does a lot more than just add the _sasl_auth variable. What 
> > is the purpose of the other changes?
> Sorry my bad, I made more changes and forgot about it when making the patch. 
> I'll make separate patches for the other changes, namely:
> 
> - added a new keyword argument to ldap2.make_filter and 
> ldap2.make_filter_from_attr, the boolean 'exact'.
> 
> If it's True, filter is build as (attribute=value), else (attribute=*value*).
> 
> - moved ldap2.__handle_errors to _handle_errors outside of the ldap2 class
> 
> It can't be in the class itself, because it is used in the _load_schema 
> function. I didn't notice this at first.
> 
> OT: I don't like using private ('__' prefix) when not absolutely necessary. I 
> think it's a little bit unpythonic. If an name starts with _, everyone knows 
> it's not indented to be used outside the scope/class it was defined in, but they 
> still have the freedom to do as they please at their own risk.
> 
> - when raising exceptions with no arguments, use 'raise ExceptionClass' instead 
> of 'raise ExceptionClass()'

I need to clarify the documentation on PublicError and its subclasses,
their intended use, because there seems to be a lot of confusion around
them.

PublicError is not just a custom exception, but also a way to relay
*translated* error messages to the user.  The translation must happen at
the time the exception is raised because each request can be in
different locale.

Among other things, PublicError.__init__() does the gettext translation
of the error message using a thread-local gettext.Translation instance
initialized with the correct local.  Not code still needs implemented,
but we should really get in the habit of raising instances instead of
classes, if your custom exception doesn't take any arguments.

I'll try clairify the docstrings on this this week.

> Well, that's just a matter of preference I guess and I only changed it to make 
> it more consistent with the way I usually write it. I'll change it back, since 
> the second form seems to be in favor in freeIPA code.
> 
> > IIRC Jason has said it is good practice to include () when raising an 
> > exception.
> > 
> > rob
> 
> Pavel
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel