[GitHub] [beam] riteshghorse commented on a change in pull request #15952: [BEAM-3304] Refactor trigger API

[GitBox <gi...@apache.org>]

riteshghorse commented on a change in pull request #15952:
URL: https://github.com/apache/beam/pull/15952#discussion_r748365829



##########
File path: sdks/go/pkg/beam/core/graph/window/trigger/trigger.go
##########
@@ -138,49 +143,114 @@ func (tr Trigger) AlignedTo(period time.Duration, offset time.Time) Trigger {
 		// TODO: Change to call UnixMilli() once we move to only supporting a go version > 1.17.
 		offsetMillis = offset.Unix()*1e3 + int64(offset.Nanosecond())/1e6
 	}
-	tr.TimestampTransforms = append(tr.TimestampTransforms, AlignToTransform{
+	tr.timestampTransforms = append(tr.timestampTransforms, AlignToTransform{
 		Period: int64(period / time.Millisecond),
 		Offset: offsetMillis,
 	})
 	return tr
 }
 
+// RepeatTrigger fires a sub-trigger repeatedly.
+type RepeatTrigger struct {
+	subTrigger Trigger
+}
+
+func (t RepeatTrigger) trigger() {}
+
+// SubTrigger returns the trigger to be repeated.
+func (t *RepeatTrigger) SubTrigger() Trigger {
+	return t.subTrigger
+}
+
 // Repeat constructs a trigger that fires a trigger repeatedly
 // once the condition has been met.
 //
 // Ex: trigger.Repeat(trigger.AfterCount(1)) is same as trigger.Always().
 func Repeat(tr Trigger) Trigger {
-	return Trigger{Kind: RepeatTrigger, SubTriggers: []Trigger{tr}}
+	return &RepeatTrigger{subTrigger: tr}
+}
+
+// AfterEndOfWindowTrigger provides option to set triggers for early and late firing.
+type AfterEndOfWindowTrigger struct {
+	earlyFiring Trigger
+	lateFiring  Trigger
+}
+
+func (t AfterEndOfWindowTrigger) trigger() {}
+
+// EarlyTrigger returns the Early Firing trigger for AfterEndOfWindowTrigger.
+func (t *AfterEndOfWindowTrigger) EarlyTrigger() Trigger {
+	return t.earlyFiring
+}
+
+// LateTrigger returns the Late Firing trigger for AfterEndOfWindowTrigger.
+func (t *AfterEndOfWindowTrigger) LateTrigger() Trigger {
+	return t.lateFiring
 }
 
 // AfterEndOfWindow constructs a trigger that is configurable for early firing
 // (before the end of window) and late firing (after the end of window).
 //
-// Default Options are: Default Trigger for EarlyFiring and No LateFiring.
-// Override it with EarlyFiring and LateFiring methods on this trigger.
-func AfterEndOfWindow() Trigger {
-	defaultEarly := Default()
-	return Trigger{Kind: AfterEndOfWindowTrigger, EarlyTrigger: &defaultEarly, LateTrigger: nil}
+// Must call EarlyFiring or LateFiring method on this trigger at the time of setting.
+func AfterEndOfWindow() *AfterEndOfWindowTrigger {
+	return &AfterEndOfWindowTrigger{earlyFiring: nil, lateFiring: nil}

Review comment:
       The idea was to return the Trigger interface from all triggers. But in doing so I wanted to make sure that the methods `EarlyFiring` and `LateFiring` is always called on `AfterEndOfWindow`. Therefore, the users will always have to call `EarlyFiring` or `LateFiring` on `AfterEndOfWindow` which returns the `Trigger` interface. 




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