FrankYang0529 commented on code in PR #75185:
URL: https://github.com/apache/airflow/pull/75185#discussion_r4338595676
##########
go-sdk/airflow/node.go:
##########
@@ -178,46 +179,90 @@ func (t *TaskRef) Before(nodes ...Node) Node { return
declareEdges(t, nodes, dir
// cleanup << extract << start.
func (t *TaskRef) After(nodes ...Node) Node { return declareEdges(t, nodes,
dirAfter) }
-// Label puts text on the edge that an edge verb declares to node, as Python's
-// loaded >> Label("when empty") >> notify_empty does:
+// LabelRef is an edge label made by [Label]. [LabelRef.On] returns a Node. To
label edges, pass
+// that Node to Before or After.
+type LabelRef struct {
+ text string
+}
+
+// Label returns a label with the given text. The Python code
+// loaded >> Label("when empty") >> notify_empty becomes:
+//
+// loaded.Before(Label("when empty").On(emptyNotice))
+//
+// The text comes before the node passed to On, so it sits between the two
tasks in the code. The
+// Airflow UI draws the label between the same two tasks in the graph.
+//
+// Label panics if text is empty or holds only white space.
+func Label(text string) *LabelRef {
+ if strings.TrimSpace(text) == "" {
+ panic("airflow.Label: got a blank label; pass the text to put
on the edge")
+ }
+ return &LabelRef{text: text}
+}
+
+// On returns node, and each node in more, as one Node. Pass that Node to an
edge verb, and the verb
+// puts the label on each edge between its receiver and one of those nodes.
With After, those edges
+// point at the receiver:
+//
+// cleaned.After(Label("always").On(loaded)) // loaded >> Label("always")
>> cleanup
//
-// loaded.Before(Label(emptyNotice, "when empty"))
+// Each Node passed to Before or After can carry its own label, so the edges
that one call declares
+// can have different labels:
//
-// The label wraps the endpoint rather than the call, so each edge of a
fan-out can carry a label
-// of its own:
+// checked.Before(Label("rows found").On(processed), Label("no
rows").On(emptyNotice))
//
-// checked.Before(Label(processed, "rows found"), Label(emptyNotice, "no
rows"))
+// When On is given more than one node, the edge to or from each of those
nodes gets the same
+// label. Python's loaded >> Label("forks") >> [notify_empty,
notify_suspicious] becomes:
//
-// A label is the Node's only in the verb it is passed to. The Node that Label
returns stands for
-// node itself, and the Node a verb returns carries no label on either side of
the next verb. So a
-// label on the receiver of a verb has no edge to land on and is dropped, the
way Python's
-// Label("x") >> b alone sets no label.
+// loaded.Before(Label("forks").On(emptyNotice, suspiciousNotice))
+//
+// A label is the Node's only in the verb it is passed to. The Node that On
returns stands for the
+// nodes themselves. The Node that a verb returns carries no label into the
next verb, either as
+// its receiver or as an argument. Before and After panic when their receiver
carries a label,
+// because that label has no edge to go on. Pass the Node that On returns only
as an argument.
+// Python's Label("x") >> b drops the label without an error instead. When
labels nest, the outer
+// label is used: loaded.Before(Label("outer").On(Label("inner").On(cleaned)))
labels the edge
+// "outer".
//
// A label declared on an edge that already carries one replaces it, as
Python's DAG.set_edge_info
// does.
//
// An [Inputs] edge is labelled by declaring it again, which is idempotent:
//
// transformed := dag.Task(transform, Inputs(extracted))
-// extracted.Before(Label(transformed, "rows"))
+// extracted.Before(Label("rows").On(transformed))
//
// A label on an edge to or from a task group stays on that edge, as
[TaskGroupRef.Before]
// describes. A label on an edge between two tasks stays on that edge too,
even when the tasks are
// in different task groups. In that case Python can replace the receiver of
the verb with a group
// that holds it, which makes the edge connect other tasks.
//
-// Label panics if node is nil or text is empty.
-func Label(node Node, text string) Node {
- if node == nil {
- panic("airflow.Label: got a nil airflow.Node")
- }
- if text == "" {
- panic("airflow.Label: got an empty label; pass the text to put
on the edge")
+// On panics if:
+// - l is nil or points to a zero LabelRef
+// - node or a node in more is nil, is a nil *TaskGroupRef, or is a struct
that embeds a Node
+//
+// On does not panic on a nil *TaskRef. The edge verb that takes the Node
panics on it instead.
+func (l *LabelRef) On(node Node, more ...Node) Node {
Review Comment:
For `Label("x").On(a).Before(b)` to fail to compile, `On` would have to
return a type without `Before` and `After`. The value from `On` still has to go
into `loaded.Before(...)`, so `Before` could no longer take `...Node`. `Before`
would need a new argument type, say `Arg`, that both a `Node` and the labelled
value implement. Code that spreads a slice of nodes would then have to build a
`[]airflow.Arg`, because Go does not convert a `[]airflow.Node` into a
`[]airflow.Arg`:
```go
var shards []airflow.Node
for _, region := range regions {
shards = append(shards, dag.Task(count, airflow.TaskSpec{TaskID:
region}))
}
loaded.Before(shards...) // compiles today, but not if Before takes
...airflow.Arg
```
I prefer to keep the run-time check on the receiver.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
