Skip to content
This repository has been archived by the owner on Oct 23, 2020. It is now read-only.

Attempt to address csv output bug with flush command #213

Merged
merged 2 commits into from
Jul 29, 2014

Conversation

m3brown
Copy link
Contributor

@m3brown m3brown commented Jul 28, 2014

This is an attempt to address #209, which describes how some CSV exports have an incorrect number of records.

It was noticed that when an export is incorrect, the last line is incomplete. Below is a sample of the last two lines:

"98.18000030517578","","9875","4.159999847412109","3280","4000","160","58300","73","South Carolina","SC","0028728","22-3887207","Ginnie Mae (GNMA)","One-to-four family dwelling (other than manufactured housing)","Not applicable","Owner-occupied as a principal dwelling","Greenville, Mauldin, Easley - SC","VA-guaranteed","Refinancing","Secured by a first lien","Not a HOEPA loan","","","","","Pickens County","Female","","","","","White","Not Hispanic or Latino","0106.00","2011","0","Male","","","","","White","Not Hispanic or Latino","Department of Housing and Urban Development","HUD","Loan originated"
"144.08999633789062","01.75","6651","9.739999771118164","2365","2649","217","73300","92","New Jersey","NJ","0027536","22-3887207","Ginnie Mae (GNMA)","One-to-four family dwelling (other than manufactured housing)","Not applicable","Owner-occupied as a principal dwelling","Allentown, Bethlehem, Easton - PA, NJ","VA-guar

In the event that this is a buffering issue, this PR adds a (flush) comand after writing the csv. I was unable to recreate the issue in my development environment, so I'm not sure this will fix the issue.

@mehtadev17
Copy link
Contributor

+1. If attempt does not fix the issue, then we will revert this change.

m3brown added a commit that referenced this pull request Jul 29, 2014
Attempt to address csv output bug with flush command
@m3brown m3brown merged commit 7e00c68 into cfpb:master Jul 29, 2014
@marcesher
Copy link
Contributor

What kind of testing did you all do to lend you to believe that this would fix the issue?

@m3brown
Copy link
Contributor Author

m3brown commented Aug 4, 2014

Unfortunately we still haven't been able to recreate the issue in our local environments, so it really was a desperate attempt/test based on the fact that the stream ends unexpectedly mid line (see the sample in my original comments).

If by what kind of testing you mean ... because this actually breaks write-csv that's a good observation. The initial commit worked (i.e. had no positive or negative impact) in dev because it was missing a parentheses. The second commit resulted in empty CSV files but I didn't notice that until after this was merged. It has since been reverted

Note - I have since noticed a way to test changes in demo without first commiting to master (i.e. bypassing jenkins) so this kind of experiment shouldn't be happening in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants