[BEAM-11108] Add a version of TextIO implemented via SDF. #13188
Conversation
|
Run Go Postcommit |
Pretty straightforward. Read and ReadSdf should be functionally identical, and likewise for ReadAll and ReadAllSdf.
|
R: @lostluck |
youngoli
changed the title
|
|
||
| // fakeReadSdfFn calls the methods in readSdfFn on a single input to simulate | ||
| // executing an SDF, and outputs all elements produced by that input. | ||
| func fakeReadSdfFn(t *testing.T, f string, size int64) []string { |
There was a problem hiding this comment.
Notionally this kind of helper is a reason to have "real" SDF actuation on the Go Direct Runner, so testing the IOs is as simple as running a go direct pipeline.
That would be a good candidate for a separate JIRA on the topic as a subtask of BEAM-11076 if it doesn't already work.
There was a problem hiding this comment.
This is available in the Direct runner, so I updated the test to use it. Not sure if I should also update the normal TextIO test too, but if so it should work just as well.
| // | ||
| // Size should be greater than 0. Otherwise there is no way to split the | ||
| // restriction and this function will return the original restriction. | ||
| func (r Restriction) SizedSplits(size int64) (splits []Restriction) { |
There was a problem hiding this comment.
Consider adding a unit test, with the stated example, a "real world" example (largely with larger numbers involved, like the 64MB chunks used later in this PR), and edge cases, like exact splits, and off by 1 errors (both sides) etc (to validate that we don't somehow end up with 0 sized splits.)
Plus some fixup.
| size: 5, | ||
| want: []Restriction{{0, 5}, {5, 10}, {10, 11}}, | ||
| }, | ||
| { |
There was a problem hiding this comment.
Consider adding an exact case too eg.
Start 7, End 17
size 5
{7, 12}, {12, 17}
IIUC the implementation correctly.
If I understand the
There was a problem hiding this comment.
Yeah, that seems like a good idea. I'll add one more case before merging.
Pretty straightforward. Read and ReadSdf should be functionally identical, and likewise for ReadAll and ReadAllSdf.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.