Orion98MC / node_rrd

A node.js native binding for RRDtool (node rrd)
97 stars 25 forks source link

Feature request: make fetch() return a promise #3

Open dzimine opened 11 years ago

dzimine commented 11 years ago

so that the client can use it like:

rrd.fetch(filename,...,function(t,d) { /* do smthing / }).error(function(e){ / deal with error / }).done(function() { / happily handle results */ });

Took me some digging to figure that the way to register "fetch is done" is to check the callback for (null,null).

dzimine commented 11 years ago

I think the better way would be doing the callback once, on complete, with all the data, or on error, with error info: callback(err, data). The problem with current way is 1) it calls JS on each line, way too often, defeating nodejs async benefits, and 2) it's hard to use in service request handler. I am still figuring out how to do it, actually.

Orion98MC commented 11 years ago

1) I will think about it. Meanwhile it is similar to how some database queries callbacks are expected to work: one callback per row (see mongodb and many more).

2) You deal with it the very same way you would with async db queries. Async database queries return a cursor on which you cycle and when the cursor pointed value is null it is time to send the results to the client. Alternatively you may stream the results to the client and only sending EOL when the cursor data is null (denoting the end of the series).

Example assuming res (http response):

var series = [];
rrd.fetch(filename, options, function (error, data) {
  if (error) return res.send({error: error});
  if (!error && !data) return res.send(series);
  series.push(data);
});
RusAlex commented 6 years ago

Hello. Seems it the rrd.fetch no longer use callback with error argument. It's either null, null or timestamp, { value: }

That seems I need to hack it to make it being promisified.