[GitHub] [beam] youngoli commented on a change in pull request #11747: [BEAM-9951] Using the builder pattern for Go synthetic config frontend

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

youngoli commented on a change in pull request #11747:
URL: https://github.com/apache/beam/pull/11747#discussion_r427004608



##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -135,27 +155,79 @@ func (fn *sourceFn) ProcessElement(rt *offsetrange.Tracker, config SourceConfig,
 	return nil
 }
 
-// DefaultSourceConfig creates a SourceConfig with intended defaults for its
-// fields. SourceConfigs should be initialized with this method.
-func DefaultSourceConfig() SourceConfig {
-	return SourceConfig{
-		NumElements:   1, // Defaults shouldn't drop elements, so at least 1.
-		InitialSplits: 1, // Defaults to 1, i.e. no initial splitting.
+// SourceConfigBuilder is used to initialize SourceConfigs. See
+// SourceConfigBuilder's methods for descriptions of the fields in a
+// SourceConfig and how they can be set. The intended approach for using this
+// builder is to begin by calling the DefaultSourceConfig function, followed by
+// calling setters, followed by calling Build.
+//
+// Usage example:
+//
+//    cfg := synthetic.DefaultSourceConfig().NumElements(5000).InitialSplits(2).Build()
+type SourceConfigBuilder struct {
+	cfg SourceConfig
+}
+
+// DefaultSourceConfig creates a SourceConfigBuilder set with intended defaults
+// for the SourceConfig fields. This function is the intended starting point for
+// initializing a SourceConfig and should always be used to create
+// SourceConfigBuilders.
+//
+// To see descriptions of the various SourceConfig fields and their defaults,
+// see the methods to SourceConfigBuilder.
+func DefaultSourceConfig() *SourceConfigBuilder {
+	return &SourceConfigBuilder{
+		cfg: SourceConfig{
+			numElements:   1, // 0 is invalid (drops elements).
+			initialSplits: 1, // 0 is invalid (drops elements).
+		},
+	}
+}
+
+// NumElements is the number of elements for the source to generate and emit.
+//
+// Valid values are in the range of [1, ...] and the default value is 1. Values
+// of 0 (and below) are invalid as they result in sources that emit no elements.
+func (b *SourceConfigBuilder) NumElements(val int) *SourceConfigBuilder {
+	b.cfg.numElements = val
+	return b
+}
+
+// InitialSplits determines the number of initial splits to perform in the
+// source's SplitRestriction method. Restrictions in synthetic sources represent
+// the number of elements being emitted, and this split is performed evenly
+// across that number of elements.
+//
+// Each resulting restriction will have at least 1 element in it, and each
+// element being emitted will be contained in exactly one restriction. That
+// means that if the desired number of splits is greater than the number of
+// elements N, then N initial restrictions will be created, each containing 1
+// element.
+//
+// Valid values are in the range of [1, ...] and the default value is 1. Values
+// of 0 (and below) are invalid as they would result in dropping elements that
+// are expected to be emitted.
+func (b *SourceConfigBuilder) InitialSplits(val int) *SourceConfigBuilder {
+	b.cfg.initialSplits = val
+	return b
+}
+
+// Build constructs the SourceConfig initialized by this builder. It also
+// performs error checking on the fields, and panics if any have been set to
+// invalid values.
+func (b *SourceConfigBuilder) Build() SourceConfig {
+	if b.cfg.initialSplits <= 0 {
+		panic(fmt.Sprintf("SourceConfig.InitialSplits must be >= 1. Got: %v", b.cfg.initialSplits))
+	}
+	if b.cfg.numElements <= 0 {
+		panic(fmt.Sprintf("SourceConfig.NumElements must be >= 1. Got: %v", b.cfg.numElements))
 	}
+	return b.cfg
 }
 
 // SourceConfig is a struct containing all the configuration options for a
-// synthetic source.
+// synthetic source. It should be created via a SourceConfigBuilder.
 type SourceConfig struct {
-	// NumElements is the number of elements for the source to generate and
-	// emit.
-	NumElements int
-
-	// InitialSplits determines the number of initial splits to perform in the
-	// source's SplitRestriction method. Note that in some edge cases, the
-	// number of splits performed might differ from this config value. Each
-	// restriction will always have one element in it, and at least one
-	// restriction will always be output, so the number of splits will be in
-	// the range of [1, N] where N is the size of the original restriction.
-	InitialSplits int
+	numElements   int

Review comment:
       Oh yea, forgot about that. I'll go with having them exported and just recommend a builder. Done.




----------------------------------------------------------------
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:
users@infra.apache.org