Closed adrienverge closed 4 years ago
Thanks for providing those cases.
I have one understanding problem with your example, though. If I understand correctly, you just want to make sure that the document exists and, if not, create it with default data.
Speaking in HTTP requests, what you propose is an unconditional PUT /db/doc {data}
and ignoring the possible conflicts. Wouldn't it be better to issue first a HEAD /db/doc
, and only if that fails to do the PUT? In case the document exists, you saved the transmission of data
. In the case, the document doesn't exist yet, you had one "useless" HEAD.
It seems that what is better depends on the latency to bandwidth ratio, the document size and the probability that a document exists.
Side note: You can use suppress in your original code to shorten it a bit:
with contextlib.suppress(aiocouch.exception.ConflictError):
await doc.save()
Hello @bmario, thanks for your answer! Just to be 100% clear: there are 2 different features shown by my single use case:
doc = Document(db, 'id', init={'key': 'val'})
,About 2, I understand your point: better do a preliminary (but lightweight) HEAD request, rather than a large PUT that could be rejected with 409.
But this does not work in all cases. For example in mine, I have to create a new (small) document, and this succeeds 99% of the times. However sometimes I can fail with a ConflictError
because of (rare) concurrency problems. In these conditions, I don't want to issue a "most-of-the-time useless" HEAD request, given that my CouchDB servers are already under pressure.
Thanks for the clarification.
1. creating and initializing a doc in one line, e.g. `doc = Document(db, 'id', init={'key': 'val'})`,
I added the proposed parameter in cda57f94a26ce18c9419192375554fbfbcfe9427. It's called
data
though to be in line with thedata
member of the Document.
I also thought a lot about the other points you brought up here.
It can be needed to create a document (without caring whether it exists or not) without having to issue an unneeded HTTP call (HEAD or GET).
From an API standpoint, I think directly use the respective constructors is indeed the way to go here. This separation allows us to clearly distinguish between the "batteries-included"-interface, which will raise errors as soon as possible, and the "I-know-what-I-am-doing" interface to optimize performance. Plus, the proposed new_doc
interface would only be a thin wrapper around that anyways.
Similarly, it would be useful to be able to create a Database object without using the Database class (= "recommended API") but without making a HTTP call.
The same reasoning as above, just use the class constructor.
2. Allow saving it _if it doesn't exists already_. Something similar to `discard_changes` and `exists_ok`, although these namings would be ambiguous here, I think.
I think the pythonic way to achieve that is indeed the usage of suppress
. And I don't have to find a good name for the parameter. 😄
All in all, my preferred way to achieve the behavior of your initial example is this:
db = Database(couch, 'db')
doc = Document(db, 'id', data={'key': 'val'})
with suppress(ConflictError):
await doc.save()
Thanks for your answer and for cda57f9.
From an API standpoint, I think directly use the respective constructors is indeed the way to go here. This separation allows us to clearly distinguish between the "batteries-included"-interface, which will raise errors as soon as possible, and the "I-know-what-I-am-doing" interface to optimize performance. Plus, the proposed new_doc interface would only be a thin wrapper around that anyways.
Yes, I agree. No problem with using Document
and Database
constructors. That's what we were doing, before discovering that it wasn't the "public API" in https://github.com/metricq/aiocouch/pull/23#discussion_r399997256. Calling these the "I-know-what-I-am-doing interface" sounds good :+1:
I think the pythonic way to achieve that is indeed the usage of suppress. And I don't have to find a good name for the parameter.
Of course there are other ways using 2+ lines (like adding with suppress(aiocouch.exception.ConflictError)
). But aiocouch provides helpers to avoid that, such as db.create(exists_ok=True)
or doc.fetch(discard_changes=True)
. I just thought it would be nice to have a similar helper here.
This follows https://github.com/metricq/aiocouch/pull/23#discussion_r400097633.
It can be needed to create a document (without caring whether it exists or not) without having to issue an unneeded HTTP call (HEAD or GET). For example, currently we need a lot of Python lines:
This could be made easier with aiocouch in two ways:
Allow setting a
Document
instance with a value, without fetching it from CouchDB, in one line. E.g.... or without using the
Document
class directly:... or even better, without
await
, because it gives the impression that I/O will be done, which is exactly what we don't want:Allow saving it if it doesn't exists already. Something similar to
discard_changes
andexists_ok
, although these namings would be ambiguous here, I think.PS: Similarly, it would be useful to be able to create a
Database
object without using theDatabase
class (= "recommended API") but without making a HTTP call.