[
https://issues.apache.org/jira/browse/BEAM-6825?focusedWorklogId=224690&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-224690
]
ASF GitHub Bot logged work on BEAM-6825:
----------------------------------------
Author: ASF GitHub Bot
Created on: 09/Apr/19 00:03
Start Date: 09/Apr/19 00:03
Worklog Time Spent: 10m
Work Description: youngoli commented on pull request #8243: [BEAM-6825]
Improve Combine error messages.
URL: https://github.com/apache/beam/pull/8243#discussion_r273278742
##########
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:
This whole section below is really repetitive, with a few small differences
in the calls to funcx.Replace. Finding a way to cut down on the repetitive
parts would make this way easier to read and reason about.
Potential idea: Wrap everything within the "if fx, ok := ..." blocks in a
function that includes a lambda, where the funcx.Replace calls are made. That
way the parts that actually vary (the Replace calls and any inputs for those)
are more obvious, and the boilerplate of checking funcx.Satisfy is pared down.
----------------------------------------------------------------
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: 224690)
Time Spent: 40m (was: 0.5h)
> 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: 40m
> 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)
