[GitHub] [beam] lostluck commented on a change in pull request #15239: [BEAM-3304] Go triggering support

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

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

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