GetDynamicNodeWorkflow endpoint

[iaroslav-ciupin]

Tracking issue

https://github.com/flyteorg/flyte/issues/

Why are the changes needed?

This new endpoint will be called by Flyte console on execution page. When workflow contains dynamic nodes, for each dynamic node, console will call this endpoint with corresponding node execution id and backend will respond with dynamic workflow closure. Its a replacement API for GetNodeExecutionData which can only be called when use clicks on task execution tab to see task Inputs/Outputs.

What changes were proposed in this pull request?

How was this patch tested?

Unit & integration tests, tested end-to-end by deploying this branch on real admin deployment.

Setup process

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

[codecov]

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (73b1d40) 58.21% compared to head (c3f0685) 58.22%.
Report is 3 commits behind head on master.

Files	Patch %	Lines
flyteadmin/pkg/rpc/adminservice/node_execution.go	0.00%	15 Missing ⚠️
...teadmin/pkg/manager/impl/node_execution_manager.go	83.78%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4689      +/-   ##
==========================================
+ Coverage   58.21%   58.22%   +0.01%     
==========================================
  Files         626      626              
  Lines       53855    53932      +77     
==========================================
+ Hits        31349    31401      +52     
- Misses      19996    20021      +25     
  Partials     2510     2510              

Flag	Coverage Δ
unittests	58.22% <65.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hamersaw]

Only top-level comment is this seems kind of odd to expose through the workflow_manager. I do not see a node_manager in flyteadmin and I'm not sure that this small update merits adding one.

[iaroslav-ciupin]

@hamersaw I was thinking between node_executions_manager(since dynamic workflow is stored inside a node execuction) and workflow_manager(since it's still a workflow even if its a dynamic one). Do you want to introduce a new manager just for this? do we even have a node entity in Flyte API? I see that each manager corresponds to a Flyte entity.

[hamersaw]

@hamersaw I was thinking between node_executions_manager(since dynamic workflow is stored inside a node execuction) and workflow_manager(since it's still a workflow even if its a dynamic one). Do you want to introduce a new manager just for this? do we even have a node entity in Flyte API? I see that each manager corresponds to a Flyte entity.

To me, it makes more sense to put this in the node_execution_manager. Lets not worry about adding anything new here. @katrogan do you have any concerns with this route?

[katrogan]

+1 to node execution manager, this is execution data produced by the node at runtime and isn't a workflow in the traditional sense (we can't launch invocations of it, it's never formally registered, etc)