Add TaskExecutionMetadata to TaskExecutionEvent #190
Conversation
go.mod
Outdated
| @@ -22,7 +22,7 @@ require ( | |||
| github.com/grpc-ecosystem/grpc-gateway v1.14.3 // indirect | |||
| github.com/imdario/mergo v0.3.8 // indirect | |||
| github.com/lyft/datacatalog v0.2.1 | |||
| github.com/lyft/flyteidl v0.18.6 | |||
| github.com/lyft/flyteidl v0.18.7-0.20200923210508-5e52ea4ac960 | |||
There was a problem hiding this comment.
I won't merge until this is an official release version
|
Also hold, I want to understand this change. This is a great change and should be done for other TaskExecution metadata as well. can we sync? |
|
I am talking WRT - flyteorg/flyte#325 |
|
@kumare3 TaskExecutionMetadata is entirely brand new as of flyteorg/flyteidl#82 |
I realize that, but I think can you think throttle listed issue as well |
|
@kumare3 sorry I replied before I saw your link, thanks for pointing out that issue |
| @@ -50,7 +50,8 @@ func trimErrorMessage(original string, maxLength int) string { | |||
| return original[0:maxLength/2] + original[len(original)-maxLength/2:] | |||
| } | |||
|
|
|||
| func ToTaskExecutionEvent(taskExecID *core.TaskExecutionIdentifier, in io.InputFilePaths, out io.OutputFilePaths, info pluginCore.PhaseInfo) (*event.TaskExecutionEvent, error) { | |||
| func ToTaskExecutionEvent(taskExecID *core.TaskExecutionIdentifier, in io.InputFilePaths, out io.OutputFilePaths, info pluginCore.PhaseInfo, | |||
There was a problem hiding this comment.
is interruptible entirely decided by node? I thought it is decided by the task execution itself?
There was a problem hiding this comment.
it can be decided by the task, see here: https://github.com/lyft/flyteidl/blob/8aff8a021c0127c273f03e0311152c349972ee46/protos/flyteidl/core/tasks.proto#L82 but is stored in the node metadata
There was a problem hiding this comment.
okay wow looking at the code though, I only see it ever initialized from the workflow-level setting https://github.com/lyft/flytepropeller/blob/master/pkg/controller/nodes/node_exec_context.go#L206,L209 😮
this should be fixed
There was a problem hiding this comment.
ohh are you saying that the node will change this setting lets say for retries etc?
There was a problem hiding this comment.
to close the loop on this:
compiler does set the interruptible bit for nodes https://github.com/lyft/flytepropeller/blob/master/pkg/compiler/transformers/k8s/node.go#L60,L68
which is indeed set correctly from the task def https://github.com/lyft/flytekit/blob/master/flytekit/common/tasks/task.py#L137
TL;DR
Add TaskExecutionMetadata to TaskExecutionEvent. At this point this only captures whether a task on a spot node or not.
Type
Are all requirements met?
Complete description
https://docs.google.com/document/d/1nqAp5-DanklNvG6OeQ3uIacZYGvf0oXioz7CboP_CsU/edit
Tracking Issue
flyteorg/flyte#520
Follow-up issue
NA