Closed sampottinger closed 11 years ago
Hey @trinary, a review would be great if you have a minute at some point. If not, just assign back to me.
This looks good to me. :+1:, merge it and deploy!
There's something broken here.
If I request a contributions doc with fewer than 365 entries, it works fine. If I ask for more than that, I get 502 and this in the logs:
events.js:72
throw er; // Unhandled 'error' event
^
TypeError: Object has no method 'toISOString'
at /home/production/co_opencampaigndata/tracer_db_facade.js:226:65
at /home/production/co_opencampaigndata/node_modules/mongodb/lib/mongodb/cursor.js:169:9
at /home/production/co_opencampaigndata/node_modules/mongodb/lib/mongodb/cursor.js:200:31
<snip>
Just dug in a little more, I think it's bad/missing data tripping us up. On expenditures, the number of records is different when it breaks. I'm going to submit a PR for the empty string test in a sec.
Shoot... I feel silly I didn't test for that. :( Also, I was using the API last night and didn't catch this. Thanks. Let me know if you want additional help / if there is anything I can do.
Thanks, Sam
I deployed the fix and it addresses some of the problem, but we're going to have to do some more validation here. There's all kinds of messy stuff in the data. If I request a sufficiently large set I'll run into something else that doesn't want to be converted. I'm a little surprised we haven't seen this more.
http://co.opencampaigndata.org/loans.json?apiKey=XXXXXX&limit=5000
Offending value: Object 2013-04-09 00:00:00 has no method 'toISOString'
http://co.opencampaigndata.org/expenditures.json?apiKey=XXXXXX&limit=5000
Offending value: Object 825178 has no method 'toISOString'
As the "TODO(samnsparky): Stream results while doing" clearly states, I would prefer to do that transform while streaming over that data but that doesn't seem to be having any major performance implications right now. Anyway, this opens #42 but that is probably a lower priority bug for now.
Closes #41.