Heya Damien,
thanks for the review.
On Apr 24, 2008, at 23:12, Damien Katz wrote:
Things looks pretty good overall. Some thoughts:

No special couchdb code should be in the config module. So the code for formating and dumping to std out and the code for creating a default couchdb configuration should still be in couch_server_sup or maybe couch_server. But not in couch_config.

Actually there should be no single "default settings" code body, each module should deal with missing configuration settings on its own. And its fine if they don't handle the case of missing settings data and just fail.

At the moment there is no "default settings code body" apart from what was in couch_server_sup already; it just feeds the ini values into the couch_config instead of variables as before. Do you suggest getting rid of that? How/where to read the ini file then? Should couch_config expose functions to read ini values for the other modules to use?




Make your changes and repost for review. Also, when submitting code for review here just go ahead an include the diffs right into the posting, it will make commenting on individual bits of code easier.

Bigger question: When upgrading the server, how to populate the defaults in an existing ini file? What do we do now?

On Apr 23, 2008, at 5:35 PM, Jan Lehnardt wrote:

Dear devs.
I made some progress in getting CouchDB to be
configurable at runtime. And I like you to try it out
and send in suggestions for improvements. The
patches can be found here:
http://pastie.textmate.org/private/evprkfb1jihq2c1fnrtm7w
apply that to trunk and here:
http://pastie.textmate.org/private/wmkxbcx7ua3l1vz76jwba
put that into a file at src/couchdb/couch_config.erl

then run ./bootstrap && ./configure && make && make install

Note that I changed the couch.ini file and you better install
CouchDB into a fresh directory.

What does this do:
There is a new module couch_config that handles storing
and retrieving configuration from the other modules in
CouchDB. On startup, it initializes itself with the values
from couch.ini. It includes a simple HTTP API as well.

It also moves code from couch_server_sup.erl to
couch_config.erl, making the former a tad simpler.

What it doesn't do:
For a new configuration value to take effect, the module
that uses it must be restarted. This does not yet happen.
So effectively, you'd still have to restart CouchDB to
change settings. This will, however, be implemented
so true runtime configuration is possible.

Another feature I plan on is committing changes back
to the couch.ini file.

In conclusion, this doesn't add any actual features at the
moment, but lays the groundwork for a set of cool features
that weren't possible otherwise. The patch aims to not
change behaviour in CouchDB, save renaming some
ini options. The only place I didn't succeed in getting
to work as it did before is the dump of configuration
variables on startup with a log level set to "debug", but
that should be an easy enough fix.

Damien suggested not starting a branch for this, but if
you'd prefer, I'm happy to set one up and maintain it
until the config work is mature enough for trunk.

I open for any suggestions here from implementation
to HTTP API, please be creative :)

Enjoy!

Jan
--
PS: Here's a teaser:

$ curl -X GET http://localhost:5984/_config/httpd/port
{"ok":true,"module":"httpd","key":"port","value":"5984"}
$ curl -X PUT http://localhost:5984/_config/httpd/port -d "5985"
{"ok":true,"module":"httpd","key":"port","value":"5985"}
$ curl -X GET http://localhost:5984/_config/httpd/port
{"ok":true,"module":"httpd","key":"port","value":"5985"}
$ curl -X DELETE http://localhost:5984/_config/httpd/port
{"ok":true,"module":"httpd","key":"port","old_value":"5985"}
$ curl -X GET http://localhost:5984/_config/httpd/port
{"ok":true,"module":"httpd","key":"port","value":"no_value"}
$ curl -X PUT http://localhost:5984/_config/httpd/port -d "5984"
{"ok":true,"module":"httpd","key":"port","value":"5984"}





Reply via email to