[Freeipa-devel] [PATCH] Add ipalib.frontend.Command method to build an entry from params with attribute=True.

[Rob Crittenden]

Pavel Zuna wrote:
> Rob Crittenden wrote:
>> Pavel Zuna wrote:
>>> Rob Crittenden wrote:
>>>> Jason Gerard DeRose wrote:
>>>>> On Fri, 2009-02-20 at 11:25 -0500, Rob Crittenden wrote:
>>>>>> Pavel Zuna wrote:
>>>>>>> Add ipalib.frontend.Command method to build an entry from params 
>>>>>>> with attribute=True
>>>>>>> Modify crud Method base classes to yield params cloned with 
>>>>>>> attribute=True.
>>>>>>>
>>>>>>> Often plugins need to build entries from params. This should make 
>>>>>>> things a bit easier.
>>>>>> How will this be used? Will a plugin author call it directly?
>>>>>>
>>>>>> rob
>>>>>
>>>>> Yes, instead of the manual work being done in a plugin's execute()
>>>>> method to assemble the entry, the plugin author makes a call to
>>>>> args_options_2_entry().  This also makes things more plugable... if 
>>>>> the
>>>>> plugin was making the entry by popping known flags (or other
>>>>> non-attributes) out of a dict, new flags/non-attributes can be added
>>>>> (like in a base class) without requiring every execute() method to be
>>>>> updated.  Same thing if new attributes are added.
>>>>
>>>> Ok, the code doesn't seem to work for me. I took the user plugin and 
>>>> added this at the top of user_add():
>>>>
>>>>     def execute(self, uid, **kw):
>>>>         assert 'uid' not in kw
>>>>         assert 'dn' not in kw
>>>>         ldap = self.api.Backend.ldap
>>>>         entry = self.args_options_2_entry(uid, **kw)
>>>>
>>>> And it fails with this truncated back trace:
>>>>
>>>>   File "/home/rcrit/redhat/merge/freeipa/ipalib/frontend.py", line 
>>>> 188, in __attributes_2_entry
>>>>     if p.attribute and p.name in kw:
>>>> AttributeError: 'str' object has no attribute 'attribute'
>>>>
>>>> At this point we don't have params anymore, they have already been 
>>>> converted, validated, etc. Unless I'm misusing this call.
>>>>
>>>> rob
>>>
>>> My mistake, I admit I didn't test it properly. I'll try not to rush 
>>> things next time.
>>> I've attached a working patch to this e-mail. Sorry for the 
>>> inconvenience.
>>
>> This works but lacks setting param.attribute=True by default. I 
>> thought that was a good idea as I think the majority of things we 
>> query for will be attributes. Can you add that part back in?
>>
>> rob
> 
> Oh, I forgot about that. It should also work for multivalue params now.
> Talking of which, there was a small bug. multivalue params required
> default values to be of the same type as the param itself instead of
> tuple. I fixed it in a separate patch.
> 
> Also, I had this idea. We could introduce a new kwarg into Param. Let's
> call it 'auto' (or 'hidden') for now. It would be a combination of
> 'required', 'autofill' and 'default', but the user wouldn't be able to
> see/set it. The point is to automatically generate static or predictable
> attributes like objectClass or dn. For example:
> 
> takes_options=(
>     #...
>     Str('objectclass',
>         multivalue=True,
>         attribute=True,
>         auto=(u'top', u'ipaUser'),
>     ),
>     #...
> )
> 
> What do you think?
> 
> Pavel
> 

Interesting idea. We'd need a way to supply a function that will fill in 
this value, and I think I'd want the function executed on the server 
side. For some object types (user, group) we pull the list of attributes 
out of LDAP. This is likely to expand in the future. This way the list 
of objectclasses is more easily configurable.

I definitely think this is something to investigate. It will help reduce 
the amount of code in the CRUD objects to almost nothing.

ack to both patches. Pushed to master.

rob