[
https://issues.apache.org/jira/browse/BEAM-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126926#comment-17126926
]
Robert Burke edited comment on BEAM-10169 at 6/5/20, 4:22 PM:
--------------------------------------------------------------
Given that a given DoFn will only be suitable for one of the ParDo* methods,
we're probably better off being less polite. And in Go, there's a strong got
before want convention at least for test outputs. Further, while we're
panicking, there's no reason we shouldn't clearly indicate the context of the
error. So I was thinking:
```DoFn \{doFnName} has \{numOutputs} outputs, but ParDo\{parDoNum} requires
\{parDoNum}. Use ParDo\{numOutputs} instead.```
Of course there's also edge cases to consider. Eg. should just print ParDo
instead of ParDo1, and if there are more than 7 outputs, then it should
recommend ParDoN instead. Having the doFnName helps localize which DoFn is
being used wrong, and the panic trace will hopefully make the caller's line
number unambiguous. Further, having the doFnName helps when some user is having
ParDo called indirectly.
This isn't perfect for all problems, but it covers most of them I think.
What do you think of that? [~codeBehindMe]
was (Author: lostluck):
Given that a given DoFn will only be suitable for one of the ParDo* methods,
we're probably better off being less polite. And in Go, there's a strong got
before want convention at least for test outputs. Further, while we're
panicking, there's no reason we shouldn't clearly indicate the context of the
error. So I was thinking:
```DoFn {doFnName} has {numOutputs} outputs, but ParDo{parDoNum} requires
{parDoNum}. Use ParDo{numOutputs} instead.```
Of course there's also edge cases to consider. Eg. should just print ParDo
instead of ParDo1, and if there are more than 7 outputs, then it should
recommend ParDoN instead. Having the doFnName helps localize which DoFn is
being used wrong, and the panic trace will hopefully make the caller's line
number unambiguous. Further, having the doFnName helps when some user is having
ParDo called indirectly.
This isn't perfect for all problems, but it covers most of them I think.
What do you think of that? [~codeBehindMe]
> ParDo* functions should declare the correct output N in their error message
> ---------------------------------------------------------------------------
>
> Key: BEAM-10169
> URL: https://issues.apache.org/jira/browse/BEAM-10169
> Project: Beam
> Issue Type: Improvement
> Components: sdk-go
> Reporter: Robert Burke
> Assignee: Aaron Tillekeratne
> Priority: P3
> Labels: noob, starter
>
> User report noted the confusion in the error if you use a DoFn with 0 outputs
> with beam.ParDo instead of beam.ParDo0.
> In that case, a panic stack trace is followed by the cryptic: "expected 1
> output. Found: []"
> We can do better.
> While we can't change the return signature dynamically (that's for ParDoN
> only), we can instead clearly indicate:
> * the DoFn in question.
> * the number of outputs the DoFn has
> * and recommend using ParDo0, ParDo, ParDo2,...ParDo7, or ParDoN, as
> appropriate.
> https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/pardo.go#L361
> would need to change as well as any of the specific cases that follow.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
