Skip to content

client.py now supports creating security docs. #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

muellermichel
Copy link

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.

handling of doc update responses without id and/or rev - this is needed for the correct handling of security docs.
@benoitc
Copy link
Owner

benoitc commented Feb 28, 2014

why not using the set_security method of the Database object?

@muellermichel
Copy link
Author

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.

@benoitc
Copy link
Owner

benoitc commented Mar 4, 2014

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.

@muellermichel
Copy link
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants