Migration from 0.0.13 not sending EOF ? about node-csv HOT 23 CLOSED

NOTE: 0.2.0 works fine, 0.2.2 has the problem.

from node-csv.

0.2.1 also has the problem.

from node-csv.

I have isolated the problem, fix coming in a second, thanks

from node-csv.

\o/ !
For the record, here's the code I'm migrating:
https://github.com/Vizzuality/CartoDB-SQL-API/blob/rs120/app/controllers/app.js#L395

from node-csv.

I just uploaded a new version, probleme is, the csv.to.stream option now use pipe and i wasn't passing the options.

Change and tests illustrating the issue is here: bfbe69e

Note concerning your code, the "end" option is true by default, this follow the node conventions.

from node-csv.

I'm closing it, feel free to re-open the issue if still present

from node-csv.

Uhm, still doesn't work, but I cannot reopen.

from node-csv.

it only happens when the input is big (I guess > 81920, thus sent in chunks)

from node-csv.

you can try yourself cloning CartoDB-SQL-API and running make && make check
The latter (make check) would fail if you use current csv version

from node-csv.

NOTE: you'll need to change toStream to to.stream when switching to current csv

from node-csv.

I just run your test, 13 are failing but none of those seems to be csv related, here are my modification:

        csv()
          .from.array(data.rows)
          .to.stream(res, {columns: columns, header: true, lineBreaks: "windows"});

from node-csv.

I get a single failure here:

1. app.test CSV format, bigger than 81920 bytes:
   The error is a timeout (2000ms exceeded)
   Same modifications.
   Can it be performance degraded incredibly between 0.0.13 and 0.2.2 ?

from node-csv.

Not that i'm aware of. The parsing algorithm is the same, and on my machine, the test passed. I'll try to make a small test to compare speeds with an older version. Could u prepare a loop that create the same input as the one in your test (you're using from.array(..) right?)

from node-csv.

The input is an array of 81920 elements of value "0" and name "fname"
in this form:

  [ { fname: 0 },
    ....
    { fname: 0 } ]

Here, is you want something quick to copy&paste:

  makeInput = function(num,name,val) {
    var ret = [];
    for (var i=0; i<num; ++i) ret.push({fname:val});
    return ret;
  };

The testcase input would be produced by:

var input = makeInput(81920, 'fname', 0);

from node-csv.

NOTE: I didn't check the output either... worth doing for sure

from node-csv.

ok, i'll look at it in 20mn, doing some speed/memory comparaison can't harm

from node-csv.

The parser was twice slower than versions 0.0.x due to the usage of bind that prevend v8 from optimisation the transform function. If interested, the responsible is here: 1728f5e

the following is now 5x times faster:

    csv()
    .from.array(data)
    .to.stream(out)

a new version 0.2.4 is available in npm

from node-csv.

Great !
I tried upgrading to 0.2.4 but:

Error: Cannot find module './stringifier'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:362:17)
    at require (module.js:378:17)
    at Object.<anonymous> (/home/src/node/apps/CartoDB-SQL-API/CartoDB-SQL-API/node_modules/csv/lib/csv.js:146:15)

Missing dependency in package.json ?

from node-csv.

some lower/uper case issue, give me a second

from node-csv.

ok, could u try again downloading 0.2.4

from node-csv.

I had to clean the npm cache to really get the new version, anyway now the "stringifier" problem is gone, but I still have a timeout for that test :/

from node-csv.

is your code with the timeout on github? if not could you push it on a branch so i can test

from node-csv.

I've just pushed it to the "develop" branch of CartoDB-SQL-API, after verifying that raising the timeout of mocha invocation helps at completing the test run. I can also handle to get the tests succeed with default timeout (2 seconds) but is unstable (edge-condition).

Never had the same problem with 0.0.13 so I think there still is a performance degradation.

from node-csv.