[
https://issues.apache.org/jira/browse/BEAM-6825?focusedWorklogId=224737&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-224737
]
ASF GitHub Bot logged work on BEAM-6825:
----------------------------------------
Author: ASF GitHub Bot
Created on: 09/Apr/19 01:15
Start Date: 09/Apr/19 01:15
Worklog Time Spent: 10m
Work Description: lostluck commented on pull request #8243: [BEAM-6825]
Improve Combine error messages.
URL: https://github.com/apache/beam/pull/8243#discussion_r273293857
##########
File path: sdks/go/pkg/beam/core/graph/fn.go
##########
@@ -281,35 +282,144 @@ func NewCombineFn(fn interface{}) (*CombineFn, error) {
// AsCombineFn converts a Fn to a CombineFn, if possible.
func AsCombineFn(fn *Fn) (*CombineFn, error) {
+ const fnKind = "graph.AsCombineFn"
if fn.methods == nil {
fn.methods = make(map[string]*funcx.Fn)
}
if fn.Fn != nil {
fn.methods[mergeAccumulatorsName] = fn.Fn
}
- if err := verifyValidNames(fn, setupName, createAccumulatorName,
addInputName, mergeAccumulatorsName, extractOutputName, compactName,
teardownName); err != nil {
+ if err := verifyValidNames(fnKind, fn, setupName,
createAccumulatorName, addInputName, mergeAccumulatorsName, extractOutputName,
compactName, teardownName); err != nil {
return nil, err
}
- if _, ok := fn.methods[mergeAccumulatorsName]; !ok {
- return nil, fmt.Errorf("failed to find %v method: %v",
mergeAccumulatorsName, fn)
+ mergeFn, ok := fn.methods[mergeAccumulatorsName]
+ if !ok {
+ return nil, fmt.Errorf("%v: failed to find required %v method
on type: %v", fnKind, mergeAccumulatorsName, fn.Name())
}
- // TODO(herohde) 5/24/2017: validate the signatures, incl. consistency.
+ // CombineFn methods must satisfy the following:
+ // CreateAccumulator func() (A, error?)
+ // AddInput func(A, I) (A, error?)
+ // MergeAccumulators func(A, A) (A, error?)
+ // ExtractOutput func(A) (O, error?)
+ // This means that the other signatures *must* match the type used in
MergeAccumulators.
+ if len(mergeFn.Ret) <= 0 {
+ return nil, fmt.Errorf("%v: %v requires at least 1 return
value. : %v", fnKind, mergeAccumulatorsName, mergeFn)
Review comment:
Done. This adds lines technically, but being able to more clearly avoid
mismatches for name+signature pairs is probably worth it.
I threw it into a loop too.
----------------------------------------------------------------
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 224737)
Time Spent: 1h (was: 50m)
> Improve pipeline construction time error messages in Go SDK.
> ------------------------------------------------------------
>
> Key: BEAM-6825
> URL: https://issues.apache.org/jira/browse/BEAM-6825
> Project: Beam
> Issue Type: New Feature
> Components: sdk-go
> Reporter: Daniel Oliveira
> Assignee: Daniel Oliveira
> Priority: Minor
> Labels: triaged
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Many error messages for common pipeline construction mistakes are unclear and
> unhelpful. They need to be improved to provide more context, especially for
> newer users. This bug tracks these error message improvements.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
