-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Go SDK]: Retrieve file size in CreateInitialRestriction in textio.Read #25535
Conversation
c521089
to
bf772a1
Compare
Codecov Report
@@ Coverage Diff @@
## master #25535 +/- ##
==========================================
- Coverage 72.66% 72.66% -0.01%
==========================================
Files 759 759
Lines 100689 100685 -4
==========================================
- Hits 73168 73160 -8
- Misses 26112 26116 +4
Partials 1409 1409
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Assigning reviewers. If you would like to opt out of this review, comment R: @lostluck for label go. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Run Go PostCommit |
sdks/go/pkg/beam/io/textio/textio.go
Outdated
// error return values, this can be done in readSdfFn.CreateInitialRestriction. | ||
func sizeFn(ctx context.Context, filename string) (string, int64, error) { | ||
// readFn reads individual lines from a text file, given a filename and a | ||
// size in bytes for that file. Implemented as an SDF to allow splitting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically it no longer relies on an input size in bytes. We can to remove that part of the comment now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the comment update!
Thank you very kindly!
Fixes #25532 and refactors
textio.Read
to retrieve the file's size inreadFn.CreateInitialRestriction
instead of in a separate DoFn.Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.