dagster
dagster copied to clipboard
dagster-io
[DS][x/n] Create ChildCondition condition
Summary & Motivation
Going to leave this as a draft for a bit.
This creates a condition which resolves to the union of any conditions set on a child.
The implementation as it is right now should change, but there is a bit of complication involved in the recursive case. Essentially, imagine you have the following setup (A -> B -> C):
A's condition: SchedulingCondition.child_condition()
B's condition: SchedulingCondition.child_condition() | SchedulingCondition.foo()
C's condition: SchedulingCondition.bar()
I think it's pretty natural to expect that A's condition would end up resolving to the result bar() | foo(). However, this would mean that we'd need some concept of a "resolved form" of B's condition so that we don't end up in a situation where we just do a naive copy of B's condition, resulting in child_condition() | foo(), which resolves to child_condition() | foo() | foo(), and so on.
This resolving operation would need to happen arbitrarily deep within a top-level condition, which means we'd need some method like with_resolved_child_condtions(). All of this seemed like a fairly large amount of machinery for a prototype PR, and the only additional ability it would enable would be the possibility of nesting these child_conditions in broader expressions.
I do think that is a useful ability, and so I think that design space should be explored in the future, but for now I think it's sufficient to just make it "illegal" to combine child_condition with any of the existing operands.
How I Tested These Changes
[!WARNING] This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more
-
#21705
👈 -
#21671 : 1 other dependent PR (#21737 )
-
#21641 : 1 other dependent PR (#21670 )
-
#21640
-
#21648
-
#21615
-
#21613
-
#21612
-
#21573
-
#21546
-
#21545
-
#21541
-
#21540
-
#21539
-
#21538
-
#21537
-
#21536
-
#21535
-
#21521
-
#21520
-
#21511 : 1 other dependent PR (#21512 )
-
#21510
-
#21508
-
#21507
-
#21505
-
#21504
-
#21503
-
#21502
-
#21501
-
#21500
-
#21499
-
master
This stack of pull requests is managed by Graphite. Learn more about stacking.
Join @OwenKephart and the rest of your teammates on Graphite