Re: Batch mode options for CouchDB 4.0

2019-10-29 Thread Tabeth Nkangoh
Given that 4.0 is not one, but two major versions away from 2.X, what is the 
reason to just not deprecate this behavior and remove it entirely? 

4.0 probably wouldn’t be out for another year at least and deprecating 
information can be included in 3.0 once out. 

I see no reason to include things in a new major release that don’t actually do 
anything personally. 

Tabeth

> On Oct 29, 2019, at 6:36 PM, Arturo GARCIA-VARGAS  
> wrote:
> 
> I got a bit curious about this one.  I see that batch=ok simply spawns the 
> update.  I am on couchdb master, and I can see the commit regarding 
> _ensure_full_commit -it's now a no-op:
> 
> commit dc054e7ddcc3ea059e1f86a7039cf86912ab1052
> Author: Nick Vatamaniuc 
> Date:   Thu Sep 26 01:33:01 2019 -0400
> ... 8< ... 8<  ... 8<  ...
> `/_ensure_full_commit` HTTP API was left as is since replicator from older 
> versions of CouchDB would call that, it just returns the start time as if 
> ensure_commit function was called.
> 
>Issue: https://github.com/apache/couchdb/issues/2165
> 
> If there is no real /_ensure_full_commit anymore, then we cannot spawn 
> anymore.  Therefore, I personally think batch=ok should be removed altogether.
> 
>> On 29/10/2019 19:10, Robert Newson wrote:
>> I am fine with returning 202 even though we blocked to complete the request.
>> B.
> Seems like a good compromise until total deprecation, as spawning is not an 
> option.
> 
>  Saludos!
> 
>  Arturo


Re: Batch mode options for CouchDB 4.0

2019-10-29 Thread Arturo GARCIA-VARGAS
I got a bit curious about this one.  I see that batch=ok simply spawns 
the update.  I am on couchdb master, and I can see the commit regarding 
_ensure_full_commit -it's now a no-op:


commit dc054e7ddcc3ea059e1f86a7039cf86912ab1052
Author: Nick Vatamaniuc 
Date:   Thu Sep 26 01:33:01 2019 -0400
... 8< ... 8<  ... 8<  ...
`/_ensure_full_commit` HTTP API was left as is since replicator from 
older versions of CouchDB would call that, it just returns the start 
time as if ensure_commit function was called.


Issue: https://github.com/apache/couchdb/issues/2165

If there is no real /_ensure_full_commit anymore, then we cannot spawn 
anymore.  Therefore, I personally think batch=ok should be removed 
altogether.



On 29/10/2019 19:10, Robert Newson wrote:
I am fine with returning 202 even though we blocked to complete the request.

B.
Seems like a good compromise until total deprecation, as spawning is not 
an option.


  Saludos!

  Arturo


Re: Batch mode options for CouchDB 4.0

2019-10-29 Thread Robert Newson
I am fine with returning 202 even though we blocked to complete the request. 

B. 

> On 29 Oct 2019, at 10:24, Mike Rhodes  wrote:
> 
> There are a two things I'd like to break down here:
> 
> 1. The non-functional behaviour of the API is changing. What was hopefully a 
> short request could now block for much longer as the client must wait for a 
> write to happen. Among other things, this affects UI latency, as well as the 
> power consumption of low-power devices. Silently changing this behaviour is 
> very hard to debug client side. This is an example where the new behaviour 
> may not be better for some use-cases.
> 2. The request is documented as returning 202 only. We are proposing changing 
> that API contract.
> 
> IMO, the HTTP response code is a fundamental part of any HTTP API, and it's 
> reasonable for clients to listen on the 202 that is documented as the only 
> possible response code in this scenario. For example, the client might want 
> to be sure CouchDB is interpreting the argument they are sending correctly.
> 
> On the question of accepting any 2XX response being desirable, I would agree 
> that perhaps it is better to be liberal in what you accept, but we need to 
> therefore be strict in what we send. CouchDB isn't great at returning 400 
> when there are mutually exclusive parameters supplied in a request, for 
> example.
> 
> If the only reason for retaining this setting is to maintain backwards API 
> compatibility, and we are not worried about API purity, returning 202 seems 
> the appropriate approach to me; it may not be "correct" but it is seemingly 
> the way of achieving the stated goal of silently dropping the param in a 
> safe(ish) manner.
> 
> -- 
> Mike.
> 
>> On Wed, 23 Oct 2019, at 13:32, Jan Lehnardt wrote:
>> 
>> 
 On 23. Oct 2019, at 14:26, Arturo GARCIA-VARGAS  
 wrote:
>>> 
>>> I guess the way I see it (and where I may be wrong) is that batch=ok will 
>>> become a deprecated use of the API.  And if we are to support a deprecated 
>>> behaviour:
>>> 
>>> 1. Behave as before because you are nice, via an explicit config enable; or
>> 
>> The point is, we would be behaving “better than before”
>> 
>>> 2. Stop doing it because it is well..., deprecated.  Update your client.
>> 
>> …and we don’t want to break client software, when we don’t have to.
>> 
>> Best
>> Jan
>> —
>>> 
>>> -A.
>>> 
>>> Again my opinion :-)
>>> 
>>> On 23/10/2019 13:19, Jan Lehnardt wrote:
> On 23. Oct 2019, at 13:56, Arturo GARCIA-VARGAS  
> wrote:
> 
> Maybe my point is not coming across correctly.
> 
> By reading the docs, a consumer would match *explicitly* to a 202 
> response, to acknowledge success.
> 
> We better be consistent and either hard-break this behaviour, or behave 
> as before, but not silently switch the behaviour, even more if the 
> operation behind is a no-op.
 I think I do understand your point, however, the nature of this API allows 
 us to argue for the best of both worlds: batch=ok today says that the 
 client is fine with letting CouchDB decide when to fully commit data. 
 Depending on the circumstances, that decision could be “immediately”, or 
 it could be “some time later”. The proposal here now suggests that we 
 switch this to be always “immediately”, but regardless of batch=ok being 
 present or not, the client doesn’t really care about that. So I don’t 
 think there is a good reason for suggesting a hard break.
 Best
 Jan
 —
> 
> Well, my opinion.
> 
> On 23/10/2019 12:50, Jan Lehnardt wrote:
>>> On 23. Oct 2019, at 13:32, Arturo GARCIA-VARGAS  
>>> wrote:
>>> 
>>> Well, a consumer would be explicitly waiting the the accept response 
>>> code like responseCode === '202' as a sign of "success".  We have 
>>> silently broken the consumer.
>>> 
>>> Granted a consumer should cater for a '201' response, but the docs 
>>> explicitly say you do not get a 201 when using batch=ok.
>> A consumer that can’t deal with different HTTP response codes already 
>> isn’t doing HTTP correctly. They could already equally receive a 400, 
>> 401, 500 or any other variety or responses, so I think we’re fine here.
>>> 
>>> On 23/10/2019 12:29, Jan Lehnardt wrote:
> On 23. Oct 2019, at 13:25, Arturo GARCIA-VARGAS 
>  wrote:
> 
> My opinion
> 
> On 23/10/2019 12:15, Jan Lehnardt wrote:
>> 
>>> On 23. Oct 2019, at 12:40, Robert Samuel Newson 
>>>  wrote:
>>> 
>>> Hi,
>>> 
>>> Just confirming my position on this. We should treat a request with 
>>> batch=ok as if the setting was not there. That is, make the same 
>>> durable commit as normal. We should therefore send a 201 Created 
>>> response code. We should continue to validate the batch setting (it 
>>> can be absent or it can 

Re: Batch mode options for CouchDB 4.0

2019-10-29 Thread Mike Rhodes
There are a two things I'd like to break down here:

1. The non-functional behaviour of the API is changing. What was hopefully a 
short request could now block for much longer as the client must wait for a 
write to happen. Among other things, this affects UI latency, as well as the 
power consumption of low-power devices. Silently changing this behaviour is 
very hard to debug client side. This is an example where the new behaviour may 
not be better for some use-cases.
2. The request is documented as returning 202 only. We are proposing changing 
that API contract.

IMO, the HTTP response code is a fundamental part of any HTTP API, and it's 
reasonable for clients to listen on the 202 that is documented as the only 
possible response code in this scenario. For example, the client might want to 
be sure CouchDB is interpreting the argument they are sending correctly.

On the question of accepting any 2XX response being desirable, I would agree 
that perhaps it is better to be liberal in what you accept, but we need to 
therefore be strict in what we send. CouchDB isn't great at returning 400 when 
there are mutually exclusive parameters supplied in a request, for example.

If the only reason for retaining this setting is to maintain backwards API 
compatibility, and we are not worried about API purity, returning 202 seems the 
appropriate approach to me; it may not be "correct" but it is seemingly the way 
of achieving the stated goal of silently dropping the param in a safe(ish) 
manner.

-- 
Mike.

On Wed, 23 Oct 2019, at 13:32, Jan Lehnardt wrote:
> 
> 
> > On 23. Oct 2019, at 14:26, Arturo GARCIA-VARGAS  
> > wrote:
> > 
> > I guess the way I see it (and where I may be wrong) is that batch=ok will 
> > become a deprecated use of the API.  And if we are to support a deprecated 
> > behaviour:
> > 
> > 1. Behave as before because you are nice, via an explicit config enable; or
> 
> The point is, we would be behaving “better than before”
> 
> > 2. Stop doing it because it is well..., deprecated.  Update your client.
> 
> …and we don’t want to break client software, when we don’t have to.
> 
> Best
> Jan
> —
> > 
> > -A.
> > 
> > Again my opinion :-)
> > 
> > On 23/10/2019 13:19, Jan Lehnardt wrote:
> >>> On 23. Oct 2019, at 13:56, Arturo GARCIA-VARGAS  
> >>> wrote:
> >>> 
> >>> Maybe my point is not coming across correctly.
> >>> 
> >>> By reading the docs, a consumer would match *explicitly* to a 202 
> >>> response, to acknowledge success.
> >>> 
> >>> We better be consistent and either hard-break this behaviour, or behave 
> >>> as before, but not silently switch the behaviour, even more if the 
> >>> operation behind is a no-op.
> >> I think I do understand your point, however, the nature of this API allows 
> >> us to argue for the best of both worlds: batch=ok today says that the 
> >> client is fine with letting CouchDB decide when to fully commit data. 
> >> Depending on the circumstances, that decision could be “immediately”, or 
> >> it could be “some time later”. The proposal here now suggests that we 
> >> switch this to be always “immediately”, but regardless of batch=ok being 
> >> present or not, the client doesn’t really care about that. So I don’t 
> >> think there is a good reason for suggesting a hard break.
> >> Best
> >> Jan
> >> —
> >>> 
> >>> Well, my opinion.
> >>> 
> >>> On 23/10/2019 12:50, Jan Lehnardt wrote:
> > On 23. Oct 2019, at 13:32, Arturo GARCIA-VARGAS  
> > wrote:
> > 
> > Well, a consumer would be explicitly waiting the the accept response 
> > code like responseCode === '202' as a sign of "success".  We have 
> > silently broken the consumer.
> > 
> > Granted a consumer should cater for a '201' response, but the docs 
> > explicitly say you do not get a 201 when using batch=ok.
>  A consumer that can’t deal with different HTTP response codes already 
>  isn’t doing HTTP correctly. They could already equally receive a 400, 
>  401, 500 or any other variety or responses, so I think we’re fine here.
> > 
> > On 23/10/2019 12:29, Jan Lehnardt wrote:
> >>> On 23. Oct 2019, at 13:25, Arturo GARCIA-VARGAS 
> >>>  wrote:
> >>> 
> >>> My opinion
> >>> 
> >>> On 23/10/2019 12:15, Jan Lehnardt wrote:
>  
> > On 23. Oct 2019, at 12:40, Robert Samuel Newson 
> >  wrote:
> > 
> > Hi,
> > 
> > Just confirming my position on this. We should treat a request with 
> > batch=ok as if the setting was not there. That is, make the same 
> > durable commit as normal. We should therefore send a 201 Created 
> > response code. We should continue to validate the batch setting (it 
> > can be absent or it can be "ok" but every other value is a 400 Bad 
> > Request).
>  
> >>> -1 from me, we should:
> >>> 1. Drop it and be consistent with the API.  Maybe warning of 
> >>> deprecation in