Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add fields to NodeExecutionEvent (#315)
## Overview This is the first of [two](unionai/flyte@master...artf/subwf-event) PRs to enable Artifacts being generated in subworkflows. The basic issue is that Artifact creation works off of events. When a workflow output is supposed to create an Artifact, the [WorkflowExecutionEvent](https://github.com/unionai/flyte/blob/0683d1851ae56d1dd95e5a5ebe6d996d8be426cb/flyteidl/protos/flyteidl/event/event.proto#L16) is what triggers the Artifacts event handler to create it. However when that same workflow is run as a subworkflow, that event is not fired. The event handler has to just use the NodeExecutionEvent. However that event misses a lot of information. In particular, it's hard to retrieve Identifier of the workflow. Admin basically needs to walk up the node execution graph to arrive at the Identifier for the workflow. The solution proposed here is to simply have propeller add the subworkflow ID, since propeller has this information already readily accessible in the node execution context. Propeller also adds a boolean to the NodeExecutionEvent indicating whether or not this node corresponds to an entity that was at some point, generated by a dynamic task. Once a dynamic task runs, it comes with its own set of task and subworkflow definitions. This boolean just indicates to the events handler to not try to look up task/workflow template information in the Admin db as they may not even have been registered. See the issue for additional information. ### Changes * Add fields `target_entity` and `is_in_dynamic_chain` to the `NodeExecutionEvent` message. The `target_entity` field is currently only filled in for subworkflows. * Add `IsInDynamicChain` to the `ImmutableParentInfo` interface. * Update `ToNodeExecutionEvent` to take a sub workflow ID, and pull out the dynamic chain boolean information. * Update `sendEvents` in array node handling to read and send the dynamic chain boolean. * Add a test for golang pointer gotcha I'm always forgetting. ## Test Plan Tested by running local single-binary in dev mode on this test [workflow](https://github.com/unionai-oss/artifact-demo/blob/9d2de673b0e5fd94310ed9c5ca076d1e966ab466/demo_vault/dyn.py), and checking the events via printing. See this file for a dump of all the node execution events [events_all.txt](https://github.com/user-attachments/files/15780680/events_all.txt). A smaller file showing just the succeeded events, with all start/end nodes removed is here: [events_filtered.txt](https://github.com/user-attachments/files/15780692/events_filtered.txt) These show the correct attributes for `target_entity` and `is_in_dynamic_chain` for all events. Events were captured and recorded using the [file publisher](unionai/flyte@master...artf/subwf-event#diff-1bb3f2f825068726c05a23a4c64d82f1f13367d203c51385c5a1c53b7e68aed1R37) added in the other PR. Post pr comments (extracting target in more places), ran through this exercise again, and added a map task in the dynamic. New workflow is [here](https://github.com/unionai-oss/artifact-demo/blob/259e1b4e5e8a7e4e6b6cfd4f04f7a87215c6cfd5/demo_vault/dyn.py). All events: [events_all.txt](https://github.com/user-attachments/files/15795427/events_all.txt) Success non-start/end events: [events_success.txt](https://github.com/user-attachments/files/15795432/events_success.txt) This change will also first be deployed to the artifacts-test tenant and vetted for end-to-end correctness. After verification, this PR will be deployed out to prod first, and the second change to actually make use of these changes a couple days later. ## Rollout Plan (if applicable) *TODO: Describe any deployment or compatibility considerations for rolling out this change.* ## Upstream Changes Should this change be upstreamed to OSS (flyteorg/flyte)? If not, please uncheck this box, which is used for auditing. Note, it is the responsibility of each developer to actually upstream their changes. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F). - [x] To be upstreamed to OSS Have a placeholder PR just for IDL #5464 ## Issue https://linear.app/unionai/issue/DX-608/fix-subworkflow-artifact-creation ## Checklist * [x] Added tests * [ ] Ran a deploy dry run and shared the terraform plan * [ ] Added logging and metrics * [ ] Updated [dashboards](https://unionai.grafana.net/dashboards) and [alerts](https://unionai.grafana.net/alerting/list) * [ ] Updated documentation
- Loading branch information