lostluck commented on a change in pull request #15239:
URL: https://github.com/apache/beam/pull/15239#discussion_r679496274
##########
File path: sdks/go/pkg/beam/core/graph/window/strategy.go
##########
@@ -16,12 +16,16 @@
// Package window contains window representation, windowing strategies and
utilities.
package window
+import (
+ pipepb "github.com/apache/beam/sdks/go/pkg/beam/model/pipeline_v1"
+)
+
// WindowingStrategy defines the types of windowing used in a pipeline and
contains
// the data to support executing a windowing strategy.
type WindowingStrategy struct {
Fn *Fn
-
// TODO(BEAM-3304): trigger support
+ Trigger *pipepb.Trigger
Review comment:
As a rule, we avoid having users have access to, or deal with, the proto
directly, including via any of the built up structs. The protos are an
implementation detail, not an interface for users to use.
This largely means building up SDK specific mechanisms that are then
translated to the proto in graphx/translate.go. Essentially, the graph/**
directories shouldn't be depending on the protos.
That said, this is fine during development as Pane plumbing and such is
being developed.
--
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]
