histrio / py-couchdb

Modern pure python CouchDB Client.
https://pycouchdb.readthedocs.org/
Other
120 stars 43 forks source link

WIP: Error checking and refactor. #12

Closed flexd closed 11 years ago

flexd commented 11 years ago

This is a bit WIP but I wanted your input!

It should work, but since the tests are not updated it might break somewhere.

I've tested db.save_bulk() and db.query() and they seem to work.

What this does is that I made a GenericError() exception, which we might want to rename to RequestError I suppose.

Whenever it gets an error doing a request it raises it, so we get things like

raceback (most recent call last):
  File "./test.py", line 23, in <module>
    db.save_bulk(a_list)
  File "/home/kristoffer/.virtualenvs/eveapi/local/lib/python2.7/site-packages/pycouchdb/client.py", line 283, in save_bulk
    (resp, results) = self.resource.post("_bulk_docs", data=data, params=params)
  File "/home/kristoffer/.virtualenvs/eveapi/local/lib/python2.7/site-packages/pycouchdb/resource.py", line 70, in post
    return self.request("POST", path, **kwargs)
  File "/home/kristoffer/.virtualenvs/eveapi/local/lib/python2.7/site-packages/pycouchdb/resource.py", line 60, in request
    raise GenericError(result)
pycouchdb.exceptions.GenericError: {u'reason': u'Document id must be a string', u'error': u'bad_request'}

Instead of just 'One or more documents were not saved'.

I think this is an improvement, but it still needs a bit of work :-)

niwinz commented 11 years ago

Hi @flexd

GenericError is a bad name for the exception, because does not represents the error type, but RequestError is not the apropiate, because, a NotFound exception is also RequestError...

GenericError convinces me more than RequestError. Besides, I would make other exceptions (NotFound, Conflict, ...) extends from a GenericError. So making a more semantic exceptions tree.

Very nice work. I like it.

flexd commented 11 years ago

Yeah I originally started making extensions from GenericError but I thought I'd just make a pull-request to hear some thoughts first :-) It was all edited in like 30 minutes so I am fairly sure I have missed something somewhere.

I am not that familiar with all the error types and codes CouchDB will return, but it wouldn't be a big problem to raise all the possible exceptions at the same place, so you can be absolutely sure that when you reach client.py there has been no errors.

As you can see it also removes a lot of repetitive code as utils.as_json() is used one place instead of everywhere.

niwinz commented 11 years ago

Please use 4 spaces instead of 2 to indent the code, if not too much trouble. Otherwise, before merge your changes, I will fix the code style. ;)

Thanks for the effort.

flexd commented 11 years ago

My vim config should have 4 spaces by default.

niwinz commented 11 years ago

Hi again.

What is the status of this? Ready for merge?

flexd commented 11 years ago

Uh, I think so. I've been using it on my website, but I've been busy with exams the past month so I haven't really used all the features.

niwinz commented 11 years ago

Thanks for your work. I now merged your pull-request with some changes on error handling.

https://github.com/niwibe/py-couchdb/commit/8eb49219547806e9a3a86dcae2d7cdae431accd7