couchdbkit icon indicating copy to clipboard operation
couchdbkit copied to clipboard

client.py now supports creating security docs.

Open muellermichel opened this issue 11 years ago • 4 comments

handling of doc update responses without id and/or rev - this is needed for the correct handling of security docs - othewise a KeyError is thrown, since putting a security document will result in a response without 'id' attribute.

this has been tested with normal as well as security docs.

muellermichel avatar Feb 19 '14 13:02 muellermichel

why not using the set_security method of the Database object?

benoitc avatar Feb 28 '14 13:02 benoitc

Thanks, I didn't know about set_security. However I still reckon that the proposed code is more robust. The current behavior relies on the response containing an id, otherwise throwing a KeyError after the doc has been saved already. Is throwing a KeyError after the save really intended behavior when someone tries to save a security document using this API? I suggest to either catch this early by throwing an exception when trying to save a security doc or using the proposed code.

muellermichel avatar Mar 04 '14 07:03 muellermichel

not sur to follow. afaik the security object stored in in couchdb is not a document, t's a JSON object stored in the db without MVC support (which is somehow really fragile). I doesn't contain any ID or revision.

If you look at the code: https://github.com/benoitc/couchdbkit/blob/master/couchdbkit/client.py#L284

You will see that couchdbkit doesn't make any check here on the result.

benoitc avatar Mar 04 '14 07:03 benoitc

I wasn't talking about the implementation of set_security, but the one of save_doc. Your current save_doc implementation throws a KeyError after a successful save of a security doc. All I'm saying is, it would be better to either check for this early by throwing a better exception before saving, or not relying on the id being there in line 513. I mean look at that branch - why do you check for 'id' being in res in the if part and then access it directly in the else part anyway?

muellermichel avatar Mar 07 '14 22:03 muellermichel