youngoli commented on a change in pull request #11188: [BEAM-3301] Adding
restriction trackers and validation.
URL: https://github.com/apache/beam/pull/11188#discussion_r396917290
##########
File path: sdks/go/pkg/beam/core/graph/fn_test.go
##########
@@ -562,7 +595,13 @@ func (fn *GoodSdf) RestrictionSize(int, RestT) float64 {
return 0
}
-// TODO(BEAM-3301): Add ProcessElement impl. when restriction trackers are in.
+func (fn *GoodSdf) CreateTracker(RestT) *RTrackerT {
+ return &RTrackerT{}
+}
+
+func (fn *GoodSdf) ProcessElement(*RTrackerT, int) int {
Review comment:
That was one of the approaches I considered. I got some feedback on it both
ways, but ultimately I didn't really like that approach because it's a bit
unintuitive for users to get a different RTracker type than what they created.
Documentation would have to do some extra legwork. Plus, it goes against the
trend in Go to have users understand what's happening with concurrency.
But anyway, I'm open to that approach and may pivot to it if it makes sense,
but for now the plan is, when we add dynamic splitting, to provide a
concurrency wrapper and have users wrap their RTrackers themselves, or just
write their own concurrency. (This is apparently also the way python does it,
so it's not completely unprecedented.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
