couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Davis <paul.joseph.da...@gmail.com>
Subject Re: New Externals: Implemented with tests
Date Mon, 18 Oct 2010 17:23:36 GMT
On Mon, Oct 18, 2010 at 12:19 PM, Adam Kocoloski <kocolosk@apache.org> wrote:
> Hi Paul, nice work.  A few small points regarding couch_os_daemons:
>
> 1) couch_os_daemons:stop() will actually restart couch_os_daemons, won't it? The child
is specified as 'permanent', so even if it stops with reason normal the couch_secondary_services
supervisor will start it again, I think.
>

I think technically because of how the daemons section is implemented,
but AFAIK, the couch_daemons would never call that. I thought I'd
copied that from the couch_query_servers pattern, but
couch_query_servers appears to have no method for actually being
stopped.

I'm not entirely certain on what the best approach is. There's nothing
that actually tries to stop the daemon, so maybe just deleting that
method would be the best bet.

> 2) Why do you make the table private?  I think protected would be a better choice.  Then
handle_call(daemon_info, ...) can just return the table handle, and the client can do whatever
inspection is desired.  It would be more useful than the simple list in my opinion.
>

daemon_info is purely a hack to allow the tests to make asserts on
internal state. In a perfect world I'd conditionally compile that only
when compiling test code, but we don't have any of the build machinery
for doing that. Using a protected table isn't out of the question, I
just never meant for it to be used normally outside the gen_server
process. Though perhaps it might be useful in the future if someone
wants to have an HTTP endpoint akin to _active_tasks to see what
daemons are alive?

> 3) If you give couch_config:register a pointer to an exported function rather than an
anonymous function it becomes much easier to hot-upgrade couch_os_daemons w/o killing couch_config
down the line.

You mean as simple as exporting something like "conifg_notify/2" and
then passing
"fun config_notify/2" to couch_config:register? If so that's simple
enough I can make the change this evening.

Thanks for the feedback.

Paul Davis

> I haven't quite finished reviewing the code, but I think this stuff should make 1.1.
 Best, Adam
>
> On Oct 12, 2010, at 12:03 AM, Paul Davis wrote:
>
>> At the urging of Randall Leeds I added another convenience.
>>
>> Your os_daemons can now register to be restarted when the
>> configuration changes. This lets os_daemon processes automatically
>> upgrade themselves if they so desire.
>>
>> Branch is at: http://github.com/davisp/couchdb/commits/new_externals/
>> The commit message has a description here:
>>
>> http://github.com/davisp/couchdb/commit/653ed6dd3dea87357a90c66b612568a5a9cb6f00
>>
>> The commit is not stable. I'm forcibly pushing so it'll change. But
>> it'll be the second most recent on the new_externals branch as long as
>> that branch is alive.
>>
>> Paul Davis
>>
>>
>> On Mon, Oct 11, 2010 at 5:45 PM, Ryan C. Hill <rch@computer.org> wrote:
>>> I'd like to see it in both as well. Exciting feature.
>>>
>>> Flexible on 1.1 inclusion though. It is brand-new code after all.
>>>
>>> -R
>>>
>>>
>>> ----
>>> From: "Robert Newson" <robert.newson@gmail.com>
>>> To: dev@couchdb.apache.org
>>> Cc:
>>> Subject: Re: New Externals: Implemented with tests
>>> Date: Mon, 11 Oct 2010 16:23:04 -0500
>>>
>>> I'd love to see it on trunk and in 1.1.
>>>
>>> B.
>>>
>>> On Mon, Oct 11, 2010 at 10:15 PM, Paul Davis
>>> <paul.joseph.davis@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
>>>>
>>>
>>>
>>>
>
>

Mime
View raw message