couchdb-erlang mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Lehnardt <...@apache.org>
Subject Re: First Couchdb Patch help
Date Thu, 08 Nov 2012 14:30:07 GMT
Hi Garran,

this is a great first contribution!

On Nov 8, 2012, at 10:16 , Garren Smith <gs@redcometlabs.com> wrote:

> 
> Hi Guys,
> 
> I want yo submit a patch to couchdb. If you request the couchdb logs (http://127.0.0.1:5984/_log)
I want the option of getting them returned in json format. This makes it much easier to work
with the logs in javascript. I've done a first attempt here - https://gist.github.com/4037682
Basically replace from line 251 onwards in file couch_httpd_misc_handlers.erl, compile, run
then curl http://127.0.0.1:5984/_log?format=json for JSON log output.
> 
> I've added the format="json" q-value for when the request wants it to be formatted in
json. Hopefully there is a better way of doing this. Some things I would like checked:
> 
> 1) Is there a easier way of formatting the code to json or a less splitting of code by
brackets etc to get the Json?

One could write a proper parser, but that is likely overkill. I haven’t looked to closely
at your solution, but we could ship this experimental and let people figure out where the
current “parser” fails, so we can fix things for subsequent releases. We should make sure
though, that then errors are handled gracefully.

> 2) Is the code formatted correctly for couchdb?

Looks good to me.

> 2) Should the json helper methods be in couch_httpd_misc_handlers.erl or couch_log.erl?

I’d keep parse_to_json() in couch_log.erl, or if it gets more complicated, make a new module
couch_log_formatter.erl or something.


> 3) Am I remotely on the right track here, I'm very new to erlang so welcome any feedback.
I will happily accept the statement "Garren you clearly are clueless, step back and let the
pros do this kind of work"?

On the contrary, looks good!


* * * 

I agree with Benoit that this whole thing should also get a streaming interface (I made a
note to that end in the source when I wrote this originally), but I think this would break
the scope of this particular patch. It can easily still be done later.

For posterity, there is some additional discussion in the GitHub Pull Request you sent:

   https://github.com/apache/couchdb/pull/39


Cheers
Jan
-- 


Mime
View raw message