cloudant / python-cloudant

A Python library for Cloudant and CouchDB
Apache License 2.0
163 stars 55 forks source link

Document context handler does not allow unspecified _id #361

Closed bradwbonn closed 6 years ago

bradwbonn commented 6 years ago

It appears that the Document context handler does not behave as described in the documentation. According to the docs, the document ID is optional to specify (allowing the database to set the _id itself.) However the library does not seem to work this way.

from cloudant.document import Document
from cloudant.client import Cloudant

db = client['testdb']
with Document(db) as doc:
    doc['field1'] = "foo"
    doc['field2'] = "bar"

Produces an exception:

File "/Users/brad/RBPItool/lib/python2.7/site-packages/cloudant/document.py", line 331, in __enter__
self.fetch()
File "/Users/brad/RBPItool/lib/python2.7/site-packages/cloudant/document.py", line 164, in fetch
raise CloudantDocumentException(101)
cloudant.error.CloudantDocumentException: A document id is required to fetch document contents. Add an _id key and value to the document and re-try.
ricellis commented 6 years ago

I think the code is behaving as designed although I can see that the way the documentation is split between the Document class and context managers makes this confusing.

The documentation you point to says:

... a Document object also provides a convenient context manager. This context manager removes having to explicitly fetch() the document ...

and the context managers documentation gives this code example:

# Performs a fetch upon entry and a save upon exit of this block
    with Document(my_database, 'julia30') as doc:
        doc['name'] = 'Julia'
        doc['age'] = 30
        doc['pets'] = ['cat', 'dog', 'frog']

so it seems the intent of the document context is always to perform a fetch/save at the start end of the context. Obviously the fetch part doesn't make much sense for a document that doesn't yet exist.

I agree that the fact that the document_id parameter is marked as optional suggests that the document context should either: i) suppress that fetch exception when the doc doesn't yet exist or ii) have it's parameters documented separately from the Document class, requiring an ID

emlaver commented 6 years ago

If users are trying to update docs using the context manager, they would catch CloudantDocumentException to verify if the document does or does not exist.

We could add an example in the documentation using try/except around the context manager to show this use case.