New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

Hey everyone,

I've gone and implemented the two features discussed in [1] with the
goal of using them to replace the current _externals API. The code is
up at [2]. I'd like to get a couple people to build and run the tests
(./bootstrap && ./configure && make dev && make check) to make sure
everything is sane. Let me know if you have any comments or
suggestions. If a couple people report back with successful test runs
I'll probably push this to trunk in the next couple days barring
someone raising a serious objection.

There's also a blog post at [3] that describes the current situation
with words and stuff.

Paul Davis

[1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
[2] http://github.com/davisp/couchdb/tree/new_externals
[3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html

Re: New Externals: Implemented with tests

[Christopher O'Connell <jw...@gmail.com>]

I think that this is a really nice improvement to externals.

As discussed far in the past, writing a robust external node required a LOT
of threading code to keep a handle on things -- the KD tree implementation I
wrote in Java was almost 80% threading code [1]. Furthermore, the fact that
it doesn't communicate over stdio should make it a lot easier to debug
external processes, as it will be possible to send data directly to the
external process when testing.

I'll be updating the externalJ library to the new paradigm in the next few
weeks.

All the best,

~ Christopher

[1] http://compu.terlicio.us/code/externalj/

On Fri, Oct 1, 2010 at 15:43, Paul Davis <pa...@gmail.com>wrote:

> Hey everyone,
>
> I've gone and implemented the two features discussed in [1] with the
> goal of using them to replace the current _externals API. The code is
> up at [2]. I'd like to get a couple people to build and run the tests
> (./bootstrap && ./configure && make dev && make check) to make sure
> everything is sane. Let me know if you have any comments or
> suggestions. If a couple people report back with successful test runs
> I'll probably push this to trunk in the next couple days barring
> someone raising a serious objection.
>
> There's also a blog post at [3] that describes the current situation
> with words and stuff.
>
> Paul Davis
>
> [1]
> http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
> [2] http://github.com/davisp/couchdb/tree/new_externals
> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>

Re: New Externals: Implemented with tests

[Robert Newson <ro...@gmail.com>]

I'd love to see it on trunk and in 1.1.

B.

On Mon, Oct 11, 2010 at 10:15 PM, Paul Davis
<pa...@gmail.com> wrote:
> Just a progress update. The HTTP proxy is now capable of running the
> entire Futon test suite with only two failing tests. The http.js test
> fails because its asserting an exact URL structure, where as running
> through the proxy adds a path component. The other failing test is the
> stats module. Because I just make CouchDB proxy to itself, the five or
> so assertions on request counts fail because each request is causing
> the stat to be incremented twice (which is to be expected).
>
> I'm quite happy with how the current status of things.
>
> What do people think about committing this? I think someone said to
> wait till after 1.1.0 is released but I'm not changing any behavior of
> the existing _external's API. Its still there completely intact so we
> wouldn't be breaking anything people may currently rely on. Adding it
> before 1.1 would give more people a chance to start using this in the
> wild with crazy broken HTTP clients to find the remaining edge cases.
>
> Feedback desired.
>
> Paul Davis
>

Re: New Externals: Implemented with tests

[Sho Fukamachi <sh...@gmail.com>]

+1 for pushing into trunk immediately and preferably including in 1.1. Important feature & I for one would like to use it ASAP.

cheers Sho


On 12/10/2010, at 8:15 AM, Paul Davis wrote:

> Just a progress update. The HTTP proxy is now capable of running the
> entire Futon test suite with only two failing tests. The http.js test
> fails because its asserting an exact URL structure, where as running
> through the proxy adds a path component. The other failing test is the
> stats module. Because I just make CouchDB proxy to itself, the five or
> so assertions on request counts fail because each request is causing
> the stat to be incremented twice (which is to be expected).
> 
> I'm quite happy with how the current status of things.
> 
> What do people think about committing this? I think someone said to
> wait till after 1.1.0 is released but I'm not changing any behavior of
> the existing _external's API. Its still there completely intact so we
> wouldn't be breaking anything people may currently rely on. Adding it
> before 1.1 would give more people a chance to start using this in the
> wild with crazy broken HTTP clients to find the remaining edge cases.
> 
> Feedback desired.
> 
> Paul Davis

Re: New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

Just a progress update. The HTTP proxy is now capable of running the
entire Futon test suite with only two failing tests. The http.js test
fails because its asserting an exact URL structure, where as running
through the proxy adds a path component. The other failing test is the
stats module. Because I just make CouchDB proxy to itself, the five or
so assertions on request counts fail because each request is causing
the stat to be incremented twice (which is to be expected).

I'm quite happy with how the current status of things.

What do people think about committing this? I think someone said to
wait till after 1.1.0 is released but I'm not changing any behavior of
the existing _external's API. Its still there completely intact so we
wouldn't be breaking anything people may currently rely on. Adding it
before 1.1 would give more people a chance to start using this in the
wild with crazy broken HTTP clients to find the remaining edge cases.

Feedback desired.

Paul Davis

Re: New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

On Mon, Oct 4, 2010 at 3:43 PM, Benoit Chesneau <bc...@gmail.com> wrote:
> On Mon, Oct 4, 2010 at 7:44 PM, Paul Davis <pa...@gmail.com> wrote:
>> On Mon, Oct 4, 2010 at 12:26 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>>> On Mon, Oct 4, 2010 at 5:49 PM, Paul Davis <pa...@gmail.com> wrote:
>>>
>>>> I'm not entirely sure about rewriting headers. For the most part I
>>>> wanted to make sure that I wasn't modifying headers in anyway. If
>>>> there's a specific set of headers that need to be handled then we can
>>>> take a look at adding that in.
>>>>
>>> At least Location need cleaning at least since you get full headers
>>> and pass them to mochiweb directly in start_response. A remote server
>>> can set a Location to /some/path but you display behind
>>> /_proxied/some/path. Also during redirection (302/303) , rewriting
>>> locaton may be needed too though redirecting isn't handled yet.
>>>
>>> I've already some code for that from couchdbproxy. Will provide a patch.
>>> - benoit
>>>
>>
>> Actually I need to update the patch. I was reading RFC 2616 the other
>> day and they specifically state that a proxy is not allowed to rewrite
>> URL paths.
>>
>> I'm also not sure about the Location headers. I'll have to go back and
>> look to see what people are doing in terms of if they're setting
>> absolute or relative URI's. Theoretically, if they're relative then it
>> should work just fine.
>>
>> Paul
>>
>
> You can have a look on these code :
> http://github.com/apache/httpd/blob/trunk/modules/proxy/mod_proxy_http.c#L1196
>
> http://github.com/mojombo/yaws/blob/master/src/yaws_revproxy.erl#L399
>
> They are rewriting Location header if needed.
>
> - benoit
>

Well, seeing as apache, nginx, and yaws all do these rewrites I reckon
I should probably play along and do the same. Thanks for the links
Benoit.

Paul Davis

Re: New Externals: Implemented with tests

[Benoit Chesneau <bc...@gmail.com>]

On Mon, Oct 4, 2010 at 7:44 PM, Paul Davis <pa...@gmail.com> wrote:
> On Mon, Oct 4, 2010 at 12:26 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>> On Mon, Oct 4, 2010 at 5:49 PM, Paul Davis <pa...@gmail.com> wrote:
>>
>>> I'm not entirely sure about rewriting headers. For the most part I
>>> wanted to make sure that I wasn't modifying headers in anyway. If
>>> there's a specific set of headers that need to be handled then we can
>>> take a look at adding that in.
>>>
>> At least Location need cleaning at least since you get full headers
>> and pass them to mochiweb directly in start_response. A remote server
>> can set a Location to /some/path but you display behind
>> /_proxied/some/path. Also during redirection (302/303) , rewriting
>> locaton may be needed too though redirecting isn't handled yet.
>>
>> I've already some code for that from couchdbproxy. Will provide a patch.
>> - benoit
>>
>
> Actually I need to update the patch. I was reading RFC 2616 the other
> day and they specifically state that a proxy is not allowed to rewrite
> URL paths.
>
> I'm also not sure about the Location headers. I'll have to go back and
> look to see what people are doing in terms of if they're setting
> absolute or relative URI's. Theoretically, if they're relative then it
> should work just fine.
>
> Paul
>

You can have a look on these code :
http://github.com/apache/httpd/blob/trunk/modules/proxy/mod_proxy_http.c#L1196

http://github.com/mojombo/yaws/blob/master/src/yaws_revproxy.erl#L399

They are rewriting Location header if needed.

- benoit

Re: New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

On Mon, Oct 4, 2010 at 12:26 PM, Benoit Chesneau <bc...@gmail.com> wrote:
> On Mon, Oct 4, 2010 at 5:49 PM, Paul Davis <pa...@gmail.com> wrote:
>
>> I'm not entirely sure about rewriting headers. For the most part I
>> wanted to make sure that I wasn't modifying headers in anyway. If
>> there's a specific set of headers that need to be handled then we can
>> take a look at adding that in.
>>
> At least Location need cleaning at least since you get full headers
> and pass them to mochiweb directly in start_response. A remote server
> can set a Location to /some/path but you display behind
> /_proxied/some/path. Also during redirection (302/303) , rewriting
> locaton may be needed too though redirecting isn't handled yet.
>
> I've already some code for that from couchdbproxy. Will provide a patch.
> - benoit
>

Actually I need to update the patch. I was reading RFC 2616 the other
day and they specifically state that a proxy is not allowed to rewrite
URL paths.

I'm also not sure about the Location headers. I'll have to go back and
look to see what people are doing in terms of if they're setting
absolute or relative URI's. Theoretically, if they're relative then it
should work just fine.

Paul

Re: New Externals: Implemented with tests

[Benoit Chesneau <bc...@gmail.com>]

On Mon, Oct 4, 2010 at 5:49 PM, Paul Davis <pa...@gmail.com> wrote:

> I'm not entirely sure about rewriting headers. For the most part I
> wanted to make sure that I wasn't modifying headers in anyway. If
> there's a specific set of headers that need to be handled then we can
> take a look at adding that in.
>
At least Location need cleaning at least since you get full headers
and pass them to mochiweb directly in start_response. A remote server
can set a Location to /some/path but you display behind
/_proxied/some/path. Also during redirection (302/303) , rewriting
locaton may be needed too though redirecting isn't handled yet.

I've already some code for that from couchdbproxy. Will provide a patch.
- benoit

Re: New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

On Sun, Oct 3, 2010 at 5:59 PM, Benoit Chesneau <bc...@gmail.com> wrote:
> On Sat, Oct 2, 2010 at 3:18 AM, Paul Davis <pa...@gmail.com> wrote:
>> Most odd. Individual tests are run in independent processes so the
>> scope of interaction should be confined to a single *.t file. 170 is
>> pretty tiny in what it does. Once it gets to test 19, the remaining
>> tests are all asserts on a record. Let me know if you find anything
>> else.
>>
>> Paul
>>
>> On Fri, Oct 1, 2010 at 8:30 PM, Randall Leeds <ra...@gmail.com> wrote:
>>> Make check hangs at 19/? in 170-os-daemons.t for me. Running it
>>> individually worked.
>>> Could be some conflict with an early test? Seems strange. No time to
>>> investigate now.
>>>
>>> Thanks for this awesome work, Paul.
>>>
>>> -Randall
>>>
>>> On Fri, Oct 1, 2010 at 15:43, Paul Davis <pa...@gmail.com> wrote:
>>>> Hey everyone,
>>>>
>>>> I've gone and implemented the two features discussed in [1] with the
>>>> goal of using them to replace the current _externals API. The code is
>>>> up at [2]. I'd like to get a couple people to build and run the tests
>>>> (./bootstrap && ./configure && make dev && make check) to make sure
>>>> everything is sane. Let me know if you have any comments or
>>>> suggestions. If a couple people report back with successful test runs
>>>> I'll probably push this to trunk in the next couple days barring
>>>> someone raising a serious objection.
>>>>
>>>> There's also a blog post at [3] that describes the current situation
>>>> with words and stuff.
>>>>
>>>> Paul Davis
>>>>
>>>> [1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
>>>> [2] http://github.com/davisp/couchdb/tree/new_externals
>>>> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>>>>
>>>
>>
>
> Heh looks like my old proxy handler. Really like the idea. Reading the
> code I see you're not rewriting the Location header  when you send the
> response neither cleaning the Host header is there any reason of this
> ? I saw lot of problem due to this especially with javascript code. I
> may have a patch for that.
>
> About removing _externals, I'm not sure. Maybe keeping old one
> available or better if possible reusing your new os process handling .
> One thing I like with using io is that you don't need http semantic
> parsing, just a json parser which ease some codes. I can do a script
> in sh . Also less latency sometimes.
>
> make check is ok on my mac. I will do more test during the day.
>
> - benoît
>

I'm not going to delete the current _externals implementation right
away. I would plan on keeping it for at least one minor release to
make sure that people have time to test and switch to the new
protocol. For people that have a good reason to keep using the stdio
protocol I plan on writing a shim that mimics the current API as close
as possible so that we don't lose any functionality.

I'm not entirely sure about rewriting headers. For the most part I
wanted to make sure that I wasn't modifying headers in anyway. If
there's a specific set of headers that need to be handled then we can
take a look at adding that in.

HTH,
Paul Davis

Re: New Externals: Implemented with tests

[Stephen Prater <st...@agrussell.com>]

I concur with Benoít that the old stdio _externals maybe should not be  
removed right away, since there are probably people using externals  
that depend on it working that way.

It's currently implemented this way, although perhaps explicit  
instructions on how you do it the OLD way, vs how you do it the NEW  
way would be helpful.

And make check works flawlessly for me.

stephen

On Oct 3, 2010, at 4:59 PM, Benoit Chesneau wrote:

> On Sat, Oct 2, 2010 at 3:18 AM, Paul Davis <paul.joseph.davis@gmail.com 
> > wrote:
>> Most odd. Individual tests are run in independent processes so the
>> scope of interaction should be confined to a single *.t file. 170 is
>> pretty tiny in what it does. Once it gets to test 19, the remaining
>> tests are all asserts on a record. Let me know if you find anything
>> else.
>>
>> Paul
>>
>> On Fri, Oct 1, 2010 at 8:30 PM, Randall Leeds <randall.leeds@gmail.com 
>> > wrote:
>>> Make check hangs at 19/? in 170-os-daemons.t for me. Running it
>>> individually worked.
>>> Could be some conflict with an early test? Seems strange. No time to
>>> investigate now.
>>>
>>> Thanks for this awesome work, Paul.
>>>
>>> -Randall
>>>
>>> On Fri, Oct 1, 2010 at 15:43, Paul Davis <paul.joseph.davis@gmail.com 
>>> > wrote:
>>>> Hey everyone,
>>>>
>>>> I've gone and implemented the two features discussed in [1] with  
>>>> the
>>>> goal of using them to replace the current _externals API. The  
>>>> code is
>>>> up at [2]. I'd like to get a couple people to build and run the  
>>>> tests
>>>> (./bootstrap && ./configure && make dev && make check) to make sure
>>>> everything is sane. Let me know if you have any comments or
>>>> suggestions. If a couple people report back with successful test  
>>>> runs
>>>> I'll probably push this to trunk in the next couple days barring
>>>> someone raising a serious objection.
>>>>
>>>> There's also a blog post at [3] that describes the current  
>>>> situation
>>>> with words and stuff.
>>>>
>>>> Paul Davis
>>>>
>>>> [1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
>>>> [2] http://github.com/davisp/couchdb/tree/new_externals
>>>> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>>>>
>>>
>>
>
> Heh looks like my old proxy handler. Really like the idea. Reading the
> code I see you're not rewriting the Location header  when you send the
> response neither cleaning the Host header is there any reason of this
> ? I saw lot of problem due to this especially with javascript code. I
> may have a patch for that.
>
> About removing _externals, I'm not sure. Maybe keeping old one
> available or better if possible reusing your new os process handling .
> One thing I like with using io is that you don't need http semantic
> parsing, just a json parser which ease some codes. I can do a script
> in sh . Also less latency sometimes.
>
> make check is ok on my mac. I will do more test during the day.
>
> - benoît
>
>

Re: New Externals: Implemented with tests

[Benoit Chesneau <bc...@gmail.com>]

On Sat, Oct 2, 2010 at 3:18 AM, Paul Davis <pa...@gmail.com> wrote:
> Most odd. Individual tests are run in independent processes so the
> scope of interaction should be confined to a single *.t file. 170 is
> pretty tiny in what it does. Once it gets to test 19, the remaining
> tests are all asserts on a record. Let me know if you find anything
> else.
>
> Paul
>
> On Fri, Oct 1, 2010 at 8:30 PM, Randall Leeds <ra...@gmail.com> wrote:
>> Make check hangs at 19/? in 170-os-daemons.t for me. Running it
>> individually worked.
>> Could be some conflict with an early test? Seems strange. No time to
>> investigate now.
>>
>> Thanks for this awesome work, Paul.
>>
>> -Randall
>>
>> On Fri, Oct 1, 2010 at 15:43, Paul Davis <pa...@gmail.com> wrote:
>>> Hey everyone,
>>>
>>> I've gone and implemented the two features discussed in [1] with the
>>> goal of using them to replace the current _externals API. The code is
>>> up at [2]. I'd like to get a couple people to build and run the tests
>>> (./bootstrap && ./configure && make dev && make check) to make sure
>>> everything is sane. Let me know if you have any comments or
>>> suggestions. If a couple people report back with successful test runs
>>> I'll probably push this to trunk in the next couple days barring
>>> someone raising a serious objection.
>>>
>>> There's also a blog post at [3] that describes the current situation
>>> with words and stuff.
>>>
>>> Paul Davis
>>>
>>> [1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
>>> [2] http://github.com/davisp/couchdb/tree/new_externals
>>> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>>>
>>
>

Heh looks like my old proxy handler. Really like the idea. Reading the
code I see you're not rewriting the Location header  when you send the
response neither cleaning the Host header is there any reason of this
? I saw lot of problem due to this especially with javascript code. I
may have a patch for that.

About removing _externals, I'm not sure. Maybe keeping old one
available or better if possible reusing your new os process handling .
One thing I like with using io is that you don't need http semantic
parsing, just a json parser which ease some codes. I can do a script
in sh . Also less latency sometimes.

make check is ok on my mac. I will do more test during the day.

- benoît

Re: New Externals: Implemented with tests

[Paul Davis <pa...@gmail.com>]

Most odd. Individual tests are run in independent processes so the
scope of interaction should be confined to a single *.t file. 170 is
pretty tiny in what it does. Once it gets to test 19, the remaining
tests are all asserts on a record. Let me know if you find anything
else.

Paul

On Fri, Oct 1, 2010 at 8:30 PM, Randall Leeds <ra...@gmail.com> wrote:
> Make check hangs at 19/? in 170-os-daemons.t for me. Running it
> individually worked.
> Could be some conflict with an early test? Seems strange. No time to
> investigate now.
>
> Thanks for this awesome work, Paul.
>
> -Randall
>
> On Fri, Oct 1, 2010 at 15:43, Paul Davis <pa...@gmail.com> wrote:
>> Hey everyone,
>>
>> I've gone and implemented the two features discussed in [1] with the
>> goal of using them to replace the current _externals API. The code is
>> up at [2]. I'd like to get a couple people to build and run the tests
>> (./bootstrap && ./configure && make dev && make check) to make sure
>> everything is sane. Let me know if you have any comments or
>> suggestions. If a couple people report back with successful test runs
>> I'll probably push this to trunk in the next couple days barring
>> someone raising a serious objection.
>>
>> There's also a blog post at [3] that describes the current situation
>> with words and stuff.
>>
>> Paul Davis
>>
>> [1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
>> [2] http://github.com/davisp/couchdb/tree/new_externals
>> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>>
>

Re: New Externals: Implemented with tests

[Randall Leeds <ra...@gmail.com>]

Make check hangs at 19/? in 170-os-daemons.t for me. Running it
individually worked.
Could be some conflict with an early test? Seems strange. No time to
investigate now.

Thanks for this awesome work, Paul.

-Randall

On Fri, Oct 1, 2010 at 15:43, Paul Davis <pa...@gmail.com> wrote:
> Hey everyone,
>
> I've gone and implemented the two features discussed in [1] with the
> goal of using them to replace the current _externals API. The code is
> up at [2]. I'd like to get a couple people to build and run the tests
> (./bootstrap && ./configure && make dev && make check) to make sure
> everything is sane. Let me know if you have any comments or
> suggestions. If a couple people report back with successful test runs
> I'll probably push this to trunk in the next couple days barring
> someone raising a serious objection.
>
> There's also a blog post at [3] that describes the current situation
> with words and stuff.
>
> Paul Davis
>
> [1] http://mail-archives.apache.org/mod_mbox/couchdb-dev/201009.mbox/%3CAANLkTinv+-eqqraCjajVxzXA=soyCo_Pk1cs8MMCVXHT@mail.gmail.com%3E
> [2] http://github.com/davisp/couchdb/tree/new_externals
> [3] http://davispj.com/2010/09/26/new-couchdb-externals-api.html
>