-
Notifications
You must be signed in to change notification settings - Fork 671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add version control for ArrayNode eventing #4165
Conversation
Signed-off-by: Daniel Rammer <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4165 +/- ##
==========================================
+ Coverage 59.00% 59.91% +0.90%
==========================================
Files 619 569 -50
Lines 52827 40999 -11828
==========================================
- Hits 31170 24563 -6607
+ Misses 19173 14053 -5120
+ Partials 2484 2383 -101
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
@@ -379,7 +380,7 @@ func TestHandleArrayNodePhaseNone(t *testing.T) { | |||
}, | |||
expectedArrayNodePhase: v1alpha1.ArrayNodePhaseExecuting, | |||
expectedTransitionPhase: handler.EPhaseRunning, | |||
expectedExternalResourcePhases: []idlcore.TaskExecution_Phase{idlcore.TaskExecution_QUEUED, idlcore.TaskExecution_QUEUED}, | |||
expectedExternalResourcePhases: []idlcore.TaskExecution_Phase{idlcore.TaskExecution_UNDEFINED, idlcore.TaskExecution_UNDEFINED}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious, why are we changing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is the scenario where the ArrayNode has not yet started. We write an single node and task event for each subnode to alert admin of it's existence - mainly so the UI can display undefined x20
to indicate that there are 20 subtasks. We previously initially reported these with the Queued
phase, because that skips input resolution downstream allowing us to just use a mocked in-memory input value from the array rather than copy each individual subnode input to a separate blobstore location. However, we can still use Queued
as the initial phase during downstream evaluations and use Undefined
here to more accurately report that these tasks have not yet started.
Tracking issue
NA
Describe your changes
Add a version control flag for ArrayNode eventing.
0
means use the existing approach (ie. populatingExternalResourceInfo
field on theTaskExecutionEvent
) as a drop-in replacement for maptask.1
means to write separateNodeExecutionEvents
for each subNode execution (linked by parent node ID) for full metadata.Check all the applicable boxes
Screenshots
NA
Note to reviewers
NA