On 07/27/2012 05:06 AM, Petr Viktorin wrote:
On 07/26/2012 10:11 PM, John Dennis wrote:
On 07/17/2012 06:47 PM, John Dennis wrote:
ipapython/dn.py:
        in docstring:  DN(arg0, ..., locked=False, first_key_match=True)
        followed by:  def __init__(self, *args, **kwds):
        and:  kwds.get('first_key_match', True)

I don't see the reason for this. Just write `def __init__(self, *args,
locked=False, first_key_match=True)` and put a proper summary in the
first line of the docstring. Same in AVA & RDN.

A valid point. I think I was trying to be too flexible/extensible.

Sorry, I was unable to make the suggested change. I tried and it
refreshed my memory as to why it was coded the way it was.

Python considers it a syntax error if keyword arguments follow an arg
list reference. e.g.

  >>> def foo(*args, bar=None):
    File "<stdin>", line 1
      def foo(*args, bar=None):
                       ^
SyntaxError: invalid syntax

I'm not sure why, but that's why the methods are coded as

def foo(*args, **kwds):

If you have a suggestion as to how to use named parameters in this
context let me know or maybe explain why it's an illegal construct (I'm
sure it's in the language reference documentation somewhere, I just
didn't take the time to research it).


You're right, I forgot this particular Python wart. It's only fixed in
Python 3.


Looking at the code further, I recommend just getting rid of
first_key_match.

I agree, that feature is not well designed. You are also correct, it's currently not utilized.

I will remove it (which should be easy to do).


First of all, first_key_match isn't used anywhere.

More importantly, the functionality is poorly designed. You can't safely
mix first_key_match DNs and non-first_key_match DNs, because then you'll
never know what the object will do.
Whether you get a single value or a list of all shouldn't depend on the
object, but should be decided by the call -- I recommend `dn['cn']`
versus `dn.get_all('cn')`.
Furthermore, first_key_match complicates the DN code unnecessarily.

This can be done a subsequent patch; don't let it hold the DN work back.



--
John Dennis <jden...@redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

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

Reply via email to