[GitHub] [beam] lostluck commented on pull request #25808: [#23106] Add periodic.Sequence and periodic.Impulse transforms to Go SDK

["lostluck (via GitHub)" <gi...@apache.org>]

lostluck commented on PR #25808:
URL: https://github.com/apache/beam/pull/25808#issuecomment-1476634278

   The drain panic is outside of your code. From [here]( https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/sdf_invokers.go#L320) which means it's invoking a Drain for a KV for some reason...  But sequenceGenDoFn isn't a KV, it does have restrictions though.
   
   OK the thing that's failing here is that a recent change to support context.Context as a parameter into all the SDF methods changed how this one executes.
   
   In particular, it moved from determining what to pass into the function based on its signature to based on what's input. But that's not how this Transform needs to work.
   
   "old code"
   https://github.com/apache/beam/blame/9fb9d2cb415e903e4222021f8848bf4420157b2a/sdks/go/pkg/beam/core/runtime/exec/sdf_invokers.go#L337
   
   Ooof. This is why I want prism to implement all of Beam... including Drains, so this stuff can be tested without involving Dataflow.
   
   --------
   
   The fix is to change the trInvoker method a little, since it's generally passing in the wrong thing anyway. https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/sdf.go#L296
   
   The problem we're running into is deciding whether the Main Input is a KV or not, so the output can be expanded out. This is something unique to the Go SDK since we didn't have generics to lean on in the original design to have a "real" KV.
   
   Basically, we are seeing a *FullValue{ Elm: Input, Elm2: Restriction}.  
   
   So the right place to change is actually what's passed into the invoker in exec/sdf.go. 
   
   There's already handling code there... Let me think about this for a bit, since I need to follow what's happening in the code...
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org