[
https://issues.apache.org/jira/browse/BEAM-8017?focusedWorklogId=497032&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-497032
]
ASF GitHub Bot logged work on BEAM-8017:
----------------------------------------
Author: ASF GitHub Bot
Created on: 08/Oct/20 01:08
Start Date: 08/Oct/20 01:08
Worklog Time Spent: 10m
Work Description: milantracy commented on a change in pull request #13028:
URL: https://github.com/apache/beam/pull/13028#discussion_r501393713
##########
File path: sdks/go/pkg/beam/core/runtime/graphx/translate.go
##########
@@ -420,33 +490,58 @@ func (m *marshaller) expandCrossLanguage(namedEdge
NamedEdge) string {
// map consumers of these outputs to the expanded transform's
outputs.
outputs := make(map[string]string)
for i, out := range edge.Output {
- m.addNode(out.To)
+ if _, err := m.addNode(out.To); err != nil {
+ return "", errors.Wrapf(err, "Failed to expand
cross language transform for edge: %v", namedEdge)
+ }
outputs[fmt.Sprintf("i%v", i)] = nodeID(out.To)
}
transform.Outputs = outputs
- transform.EnvironmentId =
ExpandedTransform(edge.External.Expanded).EnvironmentId
+ environment, err := ExpandedTransform(edge.External.Expanded)
+ if err != nil {
+ return "", errors.Wrapf(err, "Failed to expand cross
language transform for edge: %v", namedEdge)
+ }
+ transform.EnvironmentId = environment.EnvironmentId
}
m.transforms[id] = transform
- return id
+ return id, nil
}
-func (m *marshaller) expandCoGBK(edge NamedEdge) string {
+func (m *marshaller) expandCoGBK(edge NamedEdge) (string, error) {
// TODO(BEAM-490): replace once CoGBK is a primitive. For now, we have
to translate
// CoGBK with multiple PCollections as described in cogbk.go.
id := edgeID(edge.Edge)
- kvCoderID := m.coders.Add(MakeKVUnionCoder(edge.Edge))
- gbkCoderID := m.coders.Add(MakeGBKUnionCoder(edge.Edge))
+ kvCoder, err := MakeKVUnionCoder(edge.Edge)
+ if err != nil {
+ return "", errors.Wrapf(err, "Fail to expand CoGBK transform
for edge: %v", edge)
Review comment:
thanks, this is a good catch, I also create a anonymous function to
return errors from _addMultiEdge_ method rather than create a named helper
function for the same purpose.
----------------------------------------------------------------
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: 497032)
Time Spent: 40m (was: 0.5h)
> Plumb errors and remove panics from package graphx
> --------------------------------------------------
>
> Key: BEAM-8017
> URL: https://issues.apache.org/jira/browse/BEAM-8017
> Project: Beam
> Issue Type: Improvement
> Components: sdk-go
> Reporter: Robert Burke
> Assignee: Jing Chen
> Priority: P3
> Labels: Novice, beginner, noob, starter
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The graphx package, and in particular serialize.go and coder.go should be
> returning errors back up, rather than panicing when issues occur deeper when
> marshalling types. It makes errors harder to follow since there's now a less
> necessary panic trace to skip, rather than a clearly constructed error
> message.
> Not difficult, but may be tedious. Requires plumbing the errors and
> handling/wrapping them appropriately instead of using panic. Most error
> handling is presently correctly wrapped anyway.
> The graphx package as a rule is intended for beam internal use, and not part
> of the user surface, so making the API changes (which aren't backwards
> compatible) isn't the worst. Most of the affected methods are unexported.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
