Don't use HTTP request if ids list is empty

[H--o-l]

The goal is to prevent un-necessary HTTP request to the database when the list of ids is empty. It's especially useful when use in combination with Database.update_docs():

with db.update_docs([]) as bulk:
    bulk._docs.append(a_document)
    bulk._docs.append(another_document)

[H--o-l]

I'd also appreciate a new test case.

I understand, and I tried to do it: I tried to mock Database.View._get() in order to ensure it is not call with the new code. But I didn't manage to pull it off in a reasonable amount of time. But maybe my idea wasn't the right one.

(PS: If finally we choose to raise a ValueError, in that case I will able to update tests accordingly myself.)

[bmario]

I understand, and I tried to do it: I tried to mock Database.View._get() in order to ensure it is not call with the new code. But I didn't manage to pull it off in a reasonable amount of time. But maybe my idea wasn't the right one.

For now, I'd be fine with just a test calling the code path and checking that it indeed raises the ValueError. I can imagine that adding a mock would be a quite daunting task.

(PS: If finally we choose to raise a ValueError, in that case I will able to update tests accordingly myself.)

If you can't come up with a good example, why that is a bad idea, I'd say so.

[H--o-l]

Hi @bmario !

Sorry I slowly but surely release that I didn't fully understand https://github.com/metricq/aiocouch/pull/26#issue-397009862

What I mean is that the example that made me open this PR, which was the following:

with db.update_docs([]) as bulk:  # do a HTTP call that I want to avoid
    bulk._docs.append(a_document)
    bulk._docs.append(another_document)

Could have simply be

with db.create_docs() as bulk:  # no HTTP, perfect!
    bulk._docs.append(a_document)
    bulk._docs.append(another_document)

On the side of raising a ValueError when update_docs() is called with ids=[], I think that [] is a value like any other, and should not raise an error. More generally, if a function applied to a list, and raise an error in the special case were that list is empty, it makes it hard to use it combined in the middle of other transformation of that list. For example, in functional programming you can do things like that:

new_list = list.find(...).filter(...).merge(another_list).map(...)...

If one of these functions raise an error on the empty list special case, it complexity writing that kind of code a lot. I'm not sure I'm understandable, don't hesitate to tell me if you don't understand or disagree.

In the meantime I'm for closing this PR because the use case (prevent un-necessary HTTP call) can be made with create_docs() function.

Thanks for your feedback!

[bmario]

I still think that this PR has value. As you correctly brought to my attention that the following code triggers an unnecessary request:

with db.update_docs([]) as bulk:
   pass

I think that [] is a value like any other, and should not raise an error.

This argument does not convince me. The code expresses the wish to retrieve existing documents, alter them, and store the updated contents. But the set of documents is empty. That doesn't sound right.

More generally, if a function applied to a list, and raise an error in the special case were that list is empty, it makes it hard to use it combined in the middle of other transformation of that list.

Yes, this thinking led me to the alternative suggestion of returning an empty list instead of the implicit None, because, under normal conditions, the function is a generator. Never mind, TIL return in a generator does the right thing™.

So in short, just add a test, that len(db.docs([])) == 0 and I'm willing to merge this.

[H--o-l]

Hi @bmario ! Ok, that sounds good.

There is already a test for db.docs([]) here, thus I added one for db.update_docs([]). I let you see, don't hesitate to criticize.