lukecwik commented on code in PR #24508:
URL: https://github.com/apache/beam/pull/24508#discussion_r1043548621
##########
sdks/go/pkg/beam/core/runtime/exec/datasource.go:
##########
@@ -453,7 +454,7 @@ func (n *DataSource) Checkpoint() (SplitResult,
time.Duration, bool, error) {
// sent to this DataSource, and is used to be able to perform accurate splits
// even if the DataSource has not yet received all its elements. A bufSize of
// 0 or less indicates that its unknown, and so uses the current known size.
-func (n *DataSource) Split(splits []int64, frac float64, bufSize int64)
(SplitResult, error) {
+func (n *DataSource) Split(ctx context.Context, splits []int64, frac float64,
bufSize int64) (SplitResult, error) {
if n == nil {
return SplitResult{}, fmt.Errorf("failed to split at requested
splits: {%v}, DataSource not initialized", splits)
Review Comment:
I don't think this would corrupt the split so I would return `unsuccessful
split` here ditto for line 482 where `su == nil` unless both of these are
programmatic errors where they should never be `nil` but might just happen to
be so.
Effectively anything that is recoverable would make sense to return
unsuccessful split while anything unrecoverable (e.g. you mutated the read
limit for the gRPC read operation or mutated the restriction of an SDF) should
return an error so that the bundle is failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
