Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Julian Reschke
On 21.09.2010 14:49, Jon Foster wrote: ... http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav. c?revision=982016&view=markup#l2121 This is the mod_dav function that builds the 207 response from a failed PROPPATCH. It doesn't allow us to add extra XML wherever we want; we on

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Jon Foster
Julian Reschke wrote: > On 21.09.2010 14:12, Daniel Shahaf wrote: > > Julian Reschke wrote on Tue, Sep 21, 2010 at 13:46:13 +0200: > >> Hi, > >> > >> just two comments without having looked at the remainder of the discussion. > >> > >> 1) If you need to augment a standard HTTP response code with ad

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Julian Reschke
On 21.09.2010 14:12, Daniel Shahaf wrote: Julian Reschke wrote on Tue, Sep 21, 2010 at 13:46:13 +0200: Hi, just two comments without having looked at the remainder of the discussion. 1) If you need to augment a standard HTTP response code with additional information, the right thing to use is

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Daniel Shahaf
Julian Reschke wrote on Tue, Sep 21, 2010 at 13:46:13 +0200: > Hi, > > just two comments without having looked at the remainder of the discussion. > > 1) If you need to augment a standard HTTP response code with additional > information, the right thing to use is DAV:error (see >

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Julian Reschke
On 21.09.2010 13:52, Jon Foster wrote: Hi, just two comments without having looked at the remainder of the discussion. Please read my first mail in this [PATCH 3/3] thread, which explains why these choices were made. ... That's the message I read, and I didn't see any consideration of using

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Jon Foster
21 September 2010 12:46 To: Jon Foster Cc: Daniel Shahaf; Subversion Development Subject: Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code Hi, just two comments without having looked at the remainder of the discussion. 1) If you need to augment a standard HTTP response code w

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-21 Thread Julian Reschke
Hi, just two comments without having looked at the remainder of the discussion. 1) If you need to augment a standard HTTP response code with additional information, the right thing to use is DAV:error (see ). 2) Do not use 412 Pre

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Daniel Shahaf
Jon Foster wrote on Tue, Sep 21, 2010 at 00:43:13 +0100: > Hi, > > Updated patch attached. > > [[[ > Signal SVN_ERR_BAD_OLD_VALUE over DAV using a HTTP 412 status > code inside a 207 multi-status response. > > * subversion/mod_dav_svn/util.c > (dav_svn__convert_err): Map SVN_ERR_BAD_OLD_VALUE

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Jon Foster
Hi, The error code changed while I was e-mailing the patch. Here's the patch with the new error code. [[[ Signal SVN_ERR_FS_PROP_BASEVALUE_MISMATCH over DAV using a HTTP 412 status code inside a 207 multi-status response. * subversion/mod_dav_svn/util.c (dav_svn__convert_err): Map SVN_ERR_FS_

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Jon Foster
Hi, Updated patch attached. [[[ Signal SVN_ERR_BAD_OLD_VALUE over DAV using a HTTP 412 status code inside a 207 multi-status response. * subversion/mod_dav_svn/util.c (dav_svn__convert_err): Map SVN_ERR_BAD_OLD_VALUE to 412. * subversion/libsvn_ra_neon/util.c (multistatus_baton_t): New mem

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Daniel Shahaf
Jon Foster wrote on Mon, Sep 20, 2010 at 22:59:02 +0100: > Daniel Shahaf wrote: > > Jon Foster wrote on Mon, Sep 20, 2010 at 11:01:02 +0100: > > > However, there is a simpler way! The element contains > > > the HTTP error code, usually "500 Internal Server Error". So we > > > could pick a specia

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Jon Foster
Daniel Shahaf wrote: > Jon Foster wrote on Mon, Sep 20, 2010 at 11:01:02 +0100: > > However, there is a simpler way! The element contains > > the HTTP error code, usually "500 Internal Server Error". So we > > could pick a special HTTP status code to mean SVN_ERR_BAD_OLD_VALUE. > > I think of ol

RE: Terminology (was: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code)

2010-09-20 Thread Bert Huijben
> -Original Message- > From: 'Daniel Shahaf' [mailto:d...@daniel.shahaf.name] > Sent: maandag 20 september 2010 12:58 > To: Bert Huijben > Cc: 'Jon Foster'; 'Subversion Development' > Subject: Terminology (was: [PATCH 3/3] atomic-r

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Daniel Shahaf
Jon Foster wrote on Mon, Sep 20, 2010 at 11:01:02 +0100: > This patch only does mod_dav_svn and Neon. I don't want to waste > time doing both HTTP libraries if this patch is rejected. You always have the option of discussing your plans here or on IRC before starting to write the patches. :-) Fee

Re: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Daniel Shahaf
Jon Foster wrote on Mon, Sep 20, 2010 at 11:01:02 +0100: > Hi, > > For atomic-revprop, the client needs to know if the error was > SVN_ERR_BAD_OLD_VALUE or not. For ra_local and ra_svn this is > already done; this patch does it for DAV (with Neon). > > > With mod_dav, we only have 2 ways to aff

Terminology (was: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code)

2010-09-20 Thread 'Daniel Shahaf'
Bert Huijben wrote on Mon, Sep 20, 2010 at 12:17:12 +0200: > I think 'SVN_ERR_BAD_OLD_VALUE' could use a better/more generic name though. > ('Old value' is only valid in the specific function context. Maybe extend > this to something more like this 'precondition failed') In the interest of moving

RE: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Bert Huijben
> -Original Message- > From: Jon Foster [mailto:jon.fos...@cabot.co.uk] > Sent: maandag 20 september 2010 12:01 > To: Daniel Shahaf; Subversion Development > Subject: [PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code > > Hi, > > For atomic-r

[PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

2010-09-20 Thread Jon Foster
Hi, For atomic-revprop, the client needs to know if the error was SVN_ERR_BAD_OLD_VALUE or not. For ra_local and ra_svn this is already done; this patch does it for DAV (with Neon). With mod_dav, we only have 2 ways to affect the 207 multistatus response from a failed PROPPATCH: - We can set t