[Freeipa-devel] [PATCH] Bulk

[Adam Young]

On 11/03/2010 10:35 AM, Rob Crittenden wrote:
> Adam Young wrote:
>> Joint effort between me and Rob in getting this to work.
>>
>> I've tested it with the following data:
>>
>> [ayoung at ipa freeipa]$ cat ../bulk_request.json
>> {"method":"bulk","params":[[
>> {"method":"json_metadata","params":[[],{}]},
>> {"method":"user_find","params":[[],{"whoami":" true","all":"true"}]},
>> {"method":"user_show","params":[["admin"],{"all":true}],"id":4}
>> ],{}],"id":1}
>>
>>
>> Called this way:
>>
>> curl -H "Content-Type:application/json" -H "Accept:applicaton/json" -H
>> "Accept-Language:en" --negotiate -u : --cacert /etc/ipa/ca.crt -d
>> @../bulk_request.json -X POST http://localhost:8888/ipa/json
>>
>>
>>
>> This needs a test, but that will be a follow on patch.
>
> nack, I think this needs a bit more work before we push it. There are 
> no doc strings for the arguments, no documentation at all really. At a 
> minimum the example you have in this e-mail would be handy to have in 
> the plugin.

Good idea.  I'd also like to have one of the comaands you used that had 
keywords in it.


>
> Do we want to expose this to the command-line or keep it for json 
> only? I can see admins wanting to use this for the cli but I have no 
> clue how we'd pass in the arguments :-)

I think we want it for both, but we shouldn't hold up getting this in 
waiting for the CLI.  We'll have to make changes on the webUI to make 
use of it, too.

>
> This includes some apparently unrelated changes to internal.py
Yeah.  I originally had it in internal.py, and then pulled it out into 
its own function.  THe internal.py changes will go away prior to push.

>
> The List argument is actually a comma-separated list, not a python 
> list. It works, I suppose, but I'm not sure it is the right thing. In 
> fact, I'm not sure what the right param type is in this case.

I think this is a problem with the JSON marshalling.  JSON has a 
perfectly valid Array type, not sure why we are forced to go  with the 
'v1,'v2,v3'  approach when it should be ['v1','v2','v3']

>
> I know I'm the one that suggested setting an empty error on success, I 
> wonder if that is really needed. Adam, would you use this to determine 
> if you have an actual response or not?

THis is correct, I think.  We check for the presence of an error field 
to indicate an error.  An actuall RPC error gets reported by the HTTP 
return code, so it goes down a different code path for that.

>
> This code bothered me when I wrote it last night but I was in "make it 
> work" mode. There has to be a more elegant way. The incoming dict keys 
> are unicode, we need them to be strings:
>
> +                newkw = {}
> +                for k in kw:
> +                    newkw[str(k)] = kw[k]

I'm OK with this code as it is.   If you get a better idea later on, we 
can alwyas clean it up, but this looks like as straight forward a 
translation as we are going to get.
>
> rob