-
Notifications
You must be signed in to change notification settings - Fork 672
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
Fix up CRD auto-generated code #5432
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
This reverts commit 0690d8a. Signed-off-by: Eduardo Apolinario <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5432 +/- ##
==========================================
+ Coverage 60.98% 61.04% +0.06%
==========================================
Files 793 793
Lines 51313 51504 +191
==========================================
+ Hits 31294 31443 +149
- Misses 17134 17138 +4
- Partials 2885 2923 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -4,6 +4,7 @@ import ( | |||
"bytes" | |||
|
|||
"github.com/golang/protobuf/jsonpb" | |||
"google.golang.org/protobuf/proto" |
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.
different uris?
oooh this looks like a big change. We should test it well |
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
…, and Inputs Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
…cNodeStatus Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
{"kind":"flyteworkflow","apiVersion":"flyte.lyft.com/v1alpha1","metadata":{"name":"name","namespace":"namespace","creationTimestamp":null,"labels":{"domain":"domain","execution-id":"name","project":"hello","shard-key":"6","workflow-name":"core-control-flow-conditions-multiplier-2"}},"spec":{"id":"::core.control_flow.conditions.multiplier_2","nodes":{"end-node":{"id":"end-node","resources":{},"kind":"end","inputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n0","var":"o0"}}}]},"n0":{"id":"n0","name":"fractions","resources":{},"kind":"branch","branch":{"if":{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.1}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}}}},"then":"n0-n0"},"elseIf":[{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}},"rightExpression":{"comparison":{"operator":"LTE","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":10}}}}}},"then":"n0-n1"}],"elseFail":{"failedNodeId":"fractions","message":"The input must be between 0 and 10"}},"inputBindings":[{"var":".my_input","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n0":{"id":"n0-n0","name":"double","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.double\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n1":{"id":"n0-n1","name":"square","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.square\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"start-node":{"id":"start-node","resources":{},"kind":"start"}},"connections":{"n0":["end-node"],"start-node":["n0"]},"edges":{"downstream":{"n0":["end-node"],"start-node":["n0"]},"upstream":{"end-node":["n0"],"n0":["start-node"],"n0-n0":["start-node"],"n0-n1":["start-node"]}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}},"outputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n0","var":"o0"}}}]},"inputs":{"literals":{"my_input":{"scalar":{"primitive":{"floatValue":0}}}}},"executionId":{},"tasks":{"resource_type:TASK name:\"core.control_flow.conditions.double\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.double"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","double"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}},"resource_type:TASK name:\"core.control_flow.conditions.square\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.square"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","square"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}}},"node-defaults":{},"securityContext":{},"status":{"phase":0},"rawOutputDataConfig":{},"executionConfig":{"TaskPluginImpls":null,"MaxParallelism":0,"RecoveryExecution":{},"TaskResources":{"Requests":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"},"Limits":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"}},"Interruptible":null,"OverwriteCache":false,"EnvironmentVariables":null}} |
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.
it's hard to visualize the difference here, so I'll paste the relevant field. On the left and on the right respectively:
"elseFail":{"failed_node_id":"fractions","message":"The input must be between 0 and 10"}
"elseFail":{"failedNodeId":"fractions","message":"The input must be between 0 and 10"}
.
failed_node_id
is the json tag of the field in the proto message, whereas failedNodeId
is the json tag in the protobuf tag. According to golang/protobuf#1388 (comment), the generation of the json
tags was a mistake.
{"kind":"flyteworkflow","apiVersion":"flyte.lyft.com/v1alpha1","metadata":{"name":"name","namespace":"namespace","creationTimestamp":null,"labels":{"domain":"domain","execution-id":"name","project":"hello","shard-key":"6","workflow-name":"core-control-flow-conditions-multiplier-3"}},"spec":{"id":"::core.control_flow.conditions.multiplier_3","nodes":{"end-node":{"id":"end-node","resources":{},"kind":"end","inputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n1","var":"o0"}}}]},"n0":{"id":"n0","name":"fractions","resources":{},"kind":"branch","branch":{"if":{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.1}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}}}},"then":"n0-n0"},"elseIf":[{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":10}}}}}},"then":"n0-n1"}],"elseFail":{"failedNodeId":"fractions","message":"The input must be between 0 and 10"}},"inputBindings":[{"var":".my_input","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n0":{"id":"n0-n0","name":"double","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.double\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n1":{"id":"n0-n1","name":"square","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.square\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n1":{"id":"n1","name":"double","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.double\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"n0","var":"o0"}}}]},"start-node":{"id":"start-node","resources":{},"kind":"start"}},"connections":{"n0":["n1"],"n1":["end-node"],"start-node":["n0"]},"edges":{"downstream":{"n0":["n1"],"n1":["end-node"],"start-node":["n0"]},"upstream":{"end-node":["n1"],"n0":["start-node"],"n0-n0":["start-node"],"n0-n1":["start-node"],"n1":["n0"]}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}},"outputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n1","var":"o0"}}}]},"inputs":{"literals":{"my_input":{"scalar":{"primitive":{"floatValue":0}}}}},"executionId":{},"tasks":{"resource_type:TASK name:\"core.control_flow.conditions.double\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.double"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","double"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}},"resource_type:TASK name:\"core.control_flow.conditions.square\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.square"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","square"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}}},"node-defaults":{},"securityContext":{},"status":{"phase":0},"rawOutputDataConfig":{},"executionConfig":{"TaskPluginImpls":null,"MaxParallelism":0,"RecoveryExecution":{},"TaskResources":{"Requests":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"},"Limits":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"}},"Interruptible":null,"OverwriteCache":false,"EnvironmentVariables":null}} |
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.
ditto.
{"kind":"flyteworkflow","apiVersion":"flyte.lyft.com/v1alpha1","metadata":{"name":"name","namespace":"namespace","creationTimestamp":null,"labels":{"domain":"domain","execution-id":"name","project":"hello","shard-key":"6","workflow-name":"core-control-flow-conditions-nested-conditions"}},"spec":{"id":"::core.control_flow.conditions.nested_conditions","nodes":{"end-node":{"id":"end-node","resources":{},"kind":"end","inputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n0","var":"o0"}}}]},"n0":{"id":"n0","name":"fractions","resources":{},"kind":"branch","branch":{"if":{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.1}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}}}},"then":"n0-n0"},"elseIf":[{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":1}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":10}}}}}},"then":"n0-n1"}],"else":"n0-n2"},"inputBindings":[{"var":".my_input","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n0":{"id":"n0-n0","name":"inner_fractions","resources":{},"kind":"branch","branch":{"if":{"condition":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.5}}}},"then":"n0-n0-n0"},"elseIf":[{"condition":{"conjunction":{"leftExpression":{"comparison":{"operator":"GT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.5}}}},"rightExpression":{"comparison":{"operator":"LT","leftValue":{"var":".my_input"},"rightValue":{"primitive":{"floatValue":0.7}}}}}},"then":"n0-n0-n1"}],"elseFail":{"failedNodeId":"inner_fractions","message":"Only \u003c0.7 allowed"}},"inputBindings":[{"var":".my_input","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n0-n0":{"id":"n0-n0-n0","name":"double","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.double\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n0-n1":{"id":"n0-n0-n1","name":"square","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.square\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n1":{"id":"n0-n1","name":"square","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.square\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"n0-n2":{"id":"n0-n2","name":"double","resources":{},"kind":"task","task":"resource_type:TASK name:\"core.control_flow.conditions.double\"","inputBindings":[{"var":"n","binding":{"promise":{"nodeId":"start-node","var":"my_input"}}}]},"start-node":{"id":"start-node","resources":{},"kind":"start"}},"connections":{"n0":["end-node"],"start-node":["n0"]},"edges":{"downstream":{"n0":["end-node"],"start-node":["n0"]},"upstream":{"end-node":["n0"],"n0":["start-node"],"n0-n0":["start-node"],"n0-n0-n0":["start-node"],"n0-n0-n1":["start-node"],"n0-n1":["start-node"],"n0-n2":["start-node"]}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}},"outputBindings":[{"var":"o0","binding":{"promise":{"nodeId":"n0","var":"o0"}}}]},"inputs":{"literals":{"my_input":{"scalar":{"primitive":{"floatValue":0}}}}},"executionId":{},"tasks":{"resource_type:TASK name:\"core.control_flow.conditions.double\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.double"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","double"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}},"resource_type:TASK name:\"core.control_flow.conditions.square\"":{"id":{"resourceType":"TASK","name":"core.control_flow.conditions.square"},"type":"python-task","metadata":{"runtime":{"type":"FLYTE_SDK","version":"0.32.6","flavor":"python"},"retries":{}},"interface":{"inputs":{"variables":{"n":{"type":{"simple":"FLOAT"}}}},"outputs":{"variables":{"o0":{"type":{"simple":"FLOAT"}}}}},"container":{"image":"ghcr.io/flyteorg/flytecookbook:core-8b8e1a849c9adfca88049a074b10dad278f70077","args":["pyflyte-execute","--inputs","{{.input}}","--output-prefix","{{.outputPrefix}}","--raw-output-data-prefix","{{.rawOutputDataPrefix}}","--checkpoint-path","{{.checkpointOutputPrefix}}","--prev-checkpoint","{{.prevCheckpointPrefix}}","--resolver","flytekit.core.python_auto_container.default_task_resolver","--","task-module","core.control_flow.conditions","task-name","square"],"resources":{},"config":[{"key":"testKey1","value":"testValue1"},{"key":"testKey2","value":"testValue2"},{"key":"testKey3","value":"testValue3"}]}}},"node-defaults":{},"securityContext":{},"status":{"phase":0},"rawOutputDataConfig":{},"executionConfig":{"TaskPluginImpls":null,"MaxParallelism":0,"RecoveryExecution":{},"TaskResources":{"Requests":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"},"Limits":{"CPU":"0","Memory":"0","EphemeralStorage":"0","Storage":"0","GPU":"0"}},"Interruptible":null,"OverwriteCache":false,"EnvironmentVariables":null}} |
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.
ditto.
securityContext: | ||
run_as: | ||
iam_role: abc-def | ||
k8s_service_account: service-account | ||
oauth2_client: | ||
client_id: client-id | ||
client_secret: | ||
group: group | ||
group_version: group-version | ||
key: key | ||
execution_identity: execution-identity |
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 new test has securityContext
set.
// Explain that in order for this interface to be part of a CRD we need to implement a DeepCopy method as per | ||
// https://pkg.go.dev/k8s.io/gengo/examples/deepcopy-gen. | ||
DeepCopyReferenceConstructor() ReferenceConstructor |
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.
@EngHabu , as we spoke offline, we have to add this method as part of the interface.
…atus implementation of SetExecutionError Signed-off-by: Eduardo Apolinario <[email protected]>
Why are the changes needed?
Regenerating the supporting CRD code is a very infrequent operation, so much so that the instructions are not very clear (I promise I'm going to update that in a separate PR).
What changes were proposed in this pull request?
In this PR we regenerate the CRD code to work in the monorepo and fix up some of the fields we haven't modified in the last changes, including the supporting code for gate nodes.
A change worth mentioning is that in order to mention an interface in a CRD we have to provide a
DeepCopy
method, but unfortunately, the only way is to "pollute" the interface with this new method. We have such case with theStorage
interface defined in flytestdlib.How was this patch tested?
Local sandbox and unit tests.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link