lostluck commented on a change in pull request #10991: [BEAM-3301] Refactor
DoFn validation & allow specifying main inputs.
URL: https://github.com/apache/beam/pull/10991#discussion_r388386045
##########
File path: sdks/go/pkg/beam/core/graph/fn.go
##########
@@ -209,21 +209,74 @@ func (f *DoFn) RestrictionT() *reflect.Type {
// a KV or not based on the other signatures (unless we're more loose about
which
// sideinputs are present). Bind should respect that.
+// The following constants prefixed with "Main" represent possible numbers of
Review comment:
I'm wary about exporting these constants.
For one, they're untyped constants, so they're functionally the numbers
themselves.
Otherwise the "right" go way to expose them so they have meaning would be to
have an unexported type so users can't define their own, and then define the
constants.
```
type mainInputs int32
const (
MainUnknown mainInputs = -1
MainSingle mainInputs = 1
MainKV mainInputs = 2
)
```
Then any functional option configuration method can accept them to have type
safe, pre-validated input numbers.
```
func NumInputs(mi mainInputs) Option {
return func(c *config) {
c.numMainIn = mi
}
}
```
This then saves needing to have a validation error, since package users
can't define their own mainInputs.
Another alternative is to do away with the exported constants altogether,
keep the validation, but simply document that valid inputs are 1 and 2 for
singletons and KVs respectively. Either is preferable to the current approach.
Lets not lose sight that the purpose here is to pass a hint down to make the
DoFn parameters easier to analyse. Windows and EventTimes are propagated with
the main input, but don't "count" since they are easily detectable by type.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
