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

Incremental CSV destination #1294

Merged
merged 5 commits into from
Dec 12, 2020
Merged

Incremental CSV destination #1294

merged 5 commits into from
Dec 12, 2020

Conversation

ChristopheDuong
Copy link
Contributor

What

see #1239

Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

short and sweet. 👌

one question that is blocking but once we resolve, will approve. good stuff.

@michel-tricot
Copy link
Contributor

I am a bit nervous about appending to a file.
The day we want to expand that destination to GCS/S3 the current implementation won't work. Why don't we just create a new file with the incremental data an "sequence" the filenames?

@ChristopheDuong
Copy link
Contributor Author

I am a bit nervous about appending to a file.
The day we want to expand that destination to GCS/S3 the current implementation won't work. Why don't we just create a new file with the incremental data an "sequence" the filenames?

With @cgardens's suggestions, we are not appending to a file anymore but we'll always overwrite the final file as a single file (not a folder with multiple partitioned files in it)

I think this consideration can be tackled once this destination is able to actually write in GCS/S3?

(plus the source-file can read only one file at a time at the moment!)

Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@ChristopheDuong ChristopheDuong merged commit 5e0e16d into master Dec 12, 2020
@ChristopheDuong ChristopheDuong deleted the chris/incremental-csv branch December 12, 2020 11:57
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.

4 participants