damccorm commented on code in PR #35263:
URL: https://github.com/apache/beam/pull/35263#discussion_r2152915208
##########
sdks/go/pkg/beam/runners/prism/internal/stage.go:
##########
@@ -194,7 +194,8 @@ progress:
case resp = <-b.Resp:
bundleFinished = true
if b.BundleErr != nil {
- return b.BundleErr
+ // return b.BundleErr
+ panic("this should get triggered")
Review Comment:
My take here is that:
https://github.com/apache/beam/blob/2e956263496564a614d216b7f985fa5a793baad7/sdks/go/pkg/beam/runners/prism/internal/stage.go#L209-L213
should just continue execution. This seems harmless since a failed progress
report is not inherently problematic.
I think
https://github.com/apache/beam/blob/2e956263496564a614d216b7f985fa5a793baad7/sdks/go/pkg/beam/runners/prism/internal/stage.go#L225-L229
should return the error and fail the job. A failed split means that
something important went wrong, and it is hard to know the current state of the
bundle. I'd appreciate any input here though.
--
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]
