Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write can be called multiple times, so adding a way to prevent writing headers #9

Merged

Conversation

jvelasques
Copy link

For example, you can be chunk reading documents from an Elastic Search cluster, and writing as you process them.

@joaomatossilva
Copy link
Owner

Hello..

First of all. Thank you very much for the contribution. It's an awesome feeling to know that my project is actually useful for someone.

So.. no worries about the build. That's something that I need to fix.

About the change itself.
Fist, I would love to have some tests to cover your new feature ;)

Second, I'm not so sure that we need to change the public signature of the Write method in order to be called multiple times.
What if we keep that writeHeaders flag in a private field? that way we would force to just write the headers once, subsequent calls if the writeHeaders would be false, it would skip it.

What do you think?

@jvelasques
Copy link
Author

Hi there!

I was actually reluctant to do that just because it changes the flow. Not sure who'd want to write headers in the middle of the file though, so the change does make sense. I'll change this PR!

Thanks!

@joaomatossilva
Copy link
Owner

Not sure who'd want to write headers in the middle of the file

Yup. My point exactly.

@jvelasques jvelasques force-pushed the feature/prevent-multiple-writes branch from d05fbd0 to 67bcd63 Compare December 3, 2018 11:28
@jvelasques
Copy link
Author

@joaomatossilva changed! Hopefully the test makes sense 😅

Thanks!

@diogogcunha
Copy link

Just as a nice extra info on this conversation, we were able to use this to stream an Elastic Search paged result into a CSV file directly on Azure Blob never storing anything on the local file system.
It worked really nicely! Congrats on this project

@joaomatossilva
Copy link
Owner

I've fixed (or so I think) the version tag on master. If you could rebase your change on top of it would be great.

Call me on Whatzapp if you need help doing it ;)

@jvelasques jvelasques force-pushed the feature/prevent-multiple-writes branch from 67bcd63 to 86c76f6 Compare December 6, 2018 13:08
@joaomatossilva joaomatossilva merged commit 7950c11 into joaomatossilva:master Dec 6, 2018
@joaomatossilva joaomatossilva deleted the feature/prevent-multiple-writes branch December 6, 2018 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants