lostluck commented on a change in pull request #11179: [BEAM-3301] Bugfix in
DoFn validation.
URL: https://github.com/apache/beam/pull/11179#discussion_r395444326
##########
File path: sdks/go/pkg/beam/pcollection.go
##########
@@ -60,6 +60,12 @@ func (p PCollection) Type() FullType {
return p.n.Type()
}
+// OutputsKV returns whether the output of this PCollection are single value
+// elements or KV pairs.
+func (p PCollection) OutputsKV() bool {
Review comment:
1. No need to have this exported right now, since we can't make a breaking
change later. It's only used in the same package. Let's not expand the user
surface unless there are good user usages, as a broad API get confusing.
2. IsKV would be a more precise name, since PCollections are a logical
representation of all their data, not actually a source or a sink. They can
represent KV type or they aren't.
3. This isn't checking if it's a KV type, it's checking if it's a Keyed type
or not, since it's also checking if it's a CoGBK.
4. Since this is only used in the one place, it's reasonable to move the
conditional there instead of adding the one off helper method.
I'm always willing to hear other opinions!
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
