lostluck commented on code in PR #17716:
URL: https://github.com/apache/beam/pull/17716#discussion_r877353920


##########
sdks/go/pkg/beam/core/runtime/exec/datasource.go:
##########
@@ -348,6 +349,18 @@ func (n *DataSource) makeEncodeElms() func([]*FullValue) 
([][]byte, error) {
        return encodeElms
 }
 
+func getBoundedRTrackerFromRoot(root *FullValue) (sdf.BoundableRTracker, 
float64, bool) {
+       tracker, ok := 
root.Elm.(*FullValue).Elm2.(*FullValue).Elm.(sdf.BoundableRTracker)
+       if !ok {

Review Comment:
   Please add a Warning Log in the failing type assert cases. It shouldn't be 
possible for these to fail this way. TBH We should probably 2 phase it. First 
ensure it's an RTracker (and fail with the warning with the actual type with 
%T) and then check if it's Boundable. 
   
   Then if it isn't, wrap it with the "oh it must be bounded" tracker 
implementation, since Only Boundable RTrackers can be Unbounded. Makes things 
more permissive, but fails more cleanly.



##########
sdks/go/pkg/beam/core/runtime/exec/datasource.go:
##########
@@ -348,6 +349,18 @@ func (n *DataSource) makeEncodeElms() func([]*FullValue) 
([][]byte, error) {
        return encodeElms
 }
 
+func getBoundedRTrackerFromRoot(root *FullValue) (sdf.BoundableRTracker, 
float64, bool) {
+       tracker, ok := 
root.Elm.(*FullValue).Elm2.(*FullValue).Elm.(sdf.BoundableRTracker)
+       if !ok {
+               return nil, -1.0, false
+       }
+       size, ok := root.Elm2.(float64)
+       if !ok {

Review Comment:
   Same comment here. By construction, it should always be a float64, so if 
it's not, there's a real problem we need to know about and it should be logged.



-- 
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]

Reply via email to