youngoli commented on a change in pull request #11665:
URL: https://github.com/apache/beam/pull/11665#discussion_r425481703
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -33,22 +33,30 @@ import (
// generated KV<[]byte, []byte> elements.
//
// This transform accepts a PCollection of SourceConfig, where each
SourceConfig
-// determines the synthetic source's behavior for that element.
+// determines the synthetic source's behavior for that element and outputs the
+// randomly generated elements.
//
-// This transform outputs a PCollection of randomly generated
-// KV<byte[], byte[]> elements.
+// SourceConfigs are recommended to be created via the DefaultSourceConfig and
+// then sent to a beam.Create transform once modified. Example:
+//
+// cfg1 := synthetic.DefaultSourceConfig()
+// cfg1.NumElements = 1000
+// cfg2 := synthetic.DefaultSourceConfig()
Review comment:
Yeah I was thinking the same thing. I guess it technically works that
way right now because the code catches the 0 initial splits case and clamps it
to 1, but I'm still not a huge fan of allowing it to be used as a value like
that. Hence, why I used that "proper" example.
While working on the step PR I've been thinking that the most appealing
approach to me right now is using a builder so you can do
DefaultSourceConfig().NumElements(1000).Build() and have Build catch any
invalid values. It's a little over-engineered for what we have now, but I
prefer over-engineered default values that at least have a user-friendly API
over implicitly changing 0 values to the defaults we actually want ("0 initial
splits? That's invalid so I'll just set it to 1 for you.")
----------------------------------------------------------------
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]
