From ad4ed1b4ab723210b4d4dcb083b7623acea73582 Mon Sep 17 00:00:00 2001 From: Alexey Volkov Date: Mon, 16 Sep 2019 02:25:13 -0700 Subject: [PATCH] SDK - Persisting all output values Currently, the parameter output values are not saved to storage and their values are lost as soon as garbage collector removes the workflow object. This change makes is so the parameter output values are persisted. --- sdk/python/kfp/compiler/_op_to_template.py | 2 ++ sdk/python/tests/compiler/compiler_tests.py | 9 ++++++++- sdk/python/tests/compiler/testdata/basic.yaml | 3 +++ .../tests/compiler/testdata/basic_no_decorator.yaml | 3 +++ sdk/python/tests/compiler/testdata/coin.yaml | 6 ++++++ sdk/python/tests/compiler/testdata/compose.yaml | 6 ++++++ sdk/python/tests/compiler/testdata/default_value.yaml | 3 +++ sdk/python/tests/compiler/testdata/imagepullsecrets.yaml | 3 +++ sdk/python/tests/compiler/testdata/pipelineparams.yaml | 3 +++ .../tests/compiler/testdata/preemptible_tpu_gpu.yaml | 3 +++ .../tests/compiler/testdata/recursive_do_while.yaml | 9 +++++++++ sdk/python/tests/compiler/testdata/recursive_while.yaml | 9 +++++++++ sdk/python/tests/compiler/testdata/sidecar.yaml | 3 +++ sdk/python/tests/compiler/testdata/tolerations.yaml | 3 +++ sdk/python/tests/compiler/testdata/volume.yaml | 3 +++ sdk/python/tests/compiler/testdata/withparam_global.yaml | 3 +++ .../tests/compiler/testdata/withparam_global_dict.yaml | 3 +++ sdk/python/tests/compiler/testdata/withparam_output.yaml | 3 +++ .../tests/compiler/testdata/withparam_output_dict.yaml | 3 +++ 19 files changed, 79 insertions(+), 1 deletion(-) diff --git a/sdk/python/kfp/compiler/_op_to_template.py b/sdk/python/kfp/compiler/_op_to_template.py index 7476e148870..05da04ad3cd 100644 --- a/sdk/python/kfp/compiler/_op_to_template.py +++ b/sdk/python/kfp/compiler/_op_to_template.py @@ -190,6 +190,8 @@ def _op_to_template(op: BaseOp): if isinstance(op, dsl.ContainerOp): # default output artifacts output_artifact_paths = OrderedDict(op.output_artifact_paths) + # This should have been as easy as output_artifact_paths.update(op.file_outputs), but the _outputs_to_json function changes the output names and we must do the same here, so that the names are the same + output_artifact_paths.update(sorted(((param.full_name, processed_op.file_outputs[param.name]) for param in processed_op.outputs.values()), key=lambda x: x[0])) output_artifacts = [ K8sHelper.convert_k8s_obj_to_json( diff --git a/sdk/python/tests/compiler/compiler_tests.py b/sdk/python/tests/compiler/compiler_tests.py index 2fdff0b92b4..2c5fed66739 100644 --- a/sdk/python/tests/compiler/compiler_tests.py +++ b/sdk/python/tests/compiler/compiler_tests.py @@ -98,6 +98,12 @@ def my_pipeline(msg1, json, kind, msg2='value2'): ]}, 'name': 'echo', 'outputs': { + 'artifacts': [ + { + 'name': 'echo-merged', + 'path': '/tmp/message.txt', + }, + ], 'parameters': [ {'name': 'echo-merged', 'valueFrom': {'path': '/tmp/message.txt'} @@ -571,7 +577,8 @@ def _test_op_to_template_yaml(self, ops, file_base_name): compiled_template = compiler._op_to_template._op_to_template(ops) del compiled_template['name'], expected['name'] - del compiled_template['outputs']['parameters'][0]['name'], expected['outputs']['parameters'][0]['name'] + for output in compiled_template['outputs'].get('parameters', []) + compiled_template['outputs'].get('artifacts', []) + expected['outputs'].get('parameters', []) + expected['outputs'].get('artifacts', []): + del output['name'] assert compiled_template == expected def test_tolerations(self): diff --git a/sdk/python/tests/compiler/testdata/basic.yaml b/sdk/python/tests/compiler/testdata/basic.yaml index e123d601af7..b102d9eff5a 100644 --- a/sdk/python/tests/compiler/testdata/basic.yaml +++ b/sdk/python/tests/compiler/testdata/basic.yaml @@ -73,6 +73,9 @@ spec: - name: message name: get-frequent outputs: + artifacts: + - name: get-frequent-word + path: /tmp/message.txt parameters: - name: get-frequent-word valueFrom: diff --git a/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml b/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml index d4e257030a4..97401ae3622 100644 --- a/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml +++ b/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml @@ -74,6 +74,9 @@ spec: - name: message name: get-frequent outputs: + artifacts: + - name: get-frequent-word + path: /tmp/message.txt parameters: - name: get-frequent-word valueFrom: diff --git a/sdk/python/tests/compiler/testdata/coin.yaml b/sdk/python/tests/compiler/testdata/coin.yaml index 577ffb3d767..22d6a65bd13 100644 --- a/sdk/python/tests/compiler/testdata/coin.yaml +++ b/sdk/python/tests/compiler/testdata/coin.yaml @@ -91,6 +91,9 @@ spec: image: python:alpine3.6 name: flip outputs: + artifacts: + - name: flip-output + path: /tmp/output parameters: - name: flip-output valueFrom: @@ -105,6 +108,9 @@ spec: image: python:alpine3.6 name: flip-again outputs: + artifacts: + - name: flip-again-output + path: /tmp/output parameters: - name: flip-again-output valueFrom: diff --git a/sdk/python/tests/compiler/testdata/compose.yaml b/sdk/python/tests/compiler/testdata/compose.yaml index 00c49a00878..3402b71526b 100644 --- a/sdk/python/tests/compiler/testdata/compose.yaml +++ b/sdk/python/tests/compiler/testdata/compose.yaml @@ -37,6 +37,9 @@ spec: - name: url name: download outputs: + artifacts: + - name: download-downloaded + path: /tmp/results.txt parameters: - name: download-downloaded valueFrom: @@ -85,6 +88,9 @@ spec: - name: download-downloaded name: get-frequent outputs: + artifacts: + - name: get-frequent-word + path: /tmp/message.txt parameters: - name: get-frequent-word valueFrom: diff --git a/sdk/python/tests/compiler/testdata/default_value.yaml b/sdk/python/tests/compiler/testdata/default_value.yaml index afbc8bdee6a..b728c3b9def 100644 --- a/sdk/python/tests/compiler/testdata/default_value.yaml +++ b/sdk/python/tests/compiler/testdata/default_value.yaml @@ -57,6 +57,9 @@ spec: - name: url name: download outputs: + artifacts: + - name: download-downloaded + path: /tmp/results.txt parameters: - name: download-downloaded valueFrom: diff --git a/sdk/python/tests/compiler/testdata/imagepullsecrets.yaml b/sdk/python/tests/compiler/testdata/imagepullsecrets.yaml index 63185e81a56..8e5692c0f79 100644 --- a/sdk/python/tests/compiler/testdata/imagepullsecrets.yaml +++ b/sdk/python/tests/compiler/testdata/imagepullsecrets.yaml @@ -26,6 +26,9 @@ spec: - name: message name: get-frequent outputs: + artifacts: + - name: get-frequent-word + path: /tmp/message.txt parameters: - name: get-frequent-word valueFrom: diff --git a/sdk/python/tests/compiler/testdata/pipelineparams.yaml b/sdk/python/tests/compiler/testdata/pipelineparams.yaml index ece877a7ff3..2e71241b3ea 100644 --- a/sdk/python/tests/compiler/testdata/pipelineparams.yaml +++ b/sdk/python/tests/compiler/testdata/pipelineparams.yaml @@ -41,6 +41,9 @@ spec: - sh - "-c" outputs: + artifacts: + - name: download-downloaded + path: "/tmp/results.txt" parameters: - name: download-downloaded valueFrom: diff --git a/sdk/python/tests/compiler/testdata/preemptible_tpu_gpu.yaml b/sdk/python/tests/compiler/testdata/preemptible_tpu_gpu.yaml index e70079ce5cd..fc7e4cb4b48 100644 --- a/sdk/python/tests/compiler/testdata/preemptible_tpu_gpu.yaml +++ b/sdk/python/tests/compiler/testdata/preemptible_tpu_gpu.yaml @@ -25,6 +25,9 @@ spec: nodeSelector: cloud.google.com/gke-preemptible: 'true' outputs: + artifacts: + - name: flip-output + path: /tmp/output parameters: - name: flip-output valueFrom: diff --git a/sdk/python/tests/compiler/testdata/recursive_do_while.yaml b/sdk/python/tests/compiler/testdata/recursive_do_while.yaml index f7c2f4f0f25..7a66f65c9fc 100644 --- a/sdk/python/tests/compiler/testdata/recursive_do_while.yaml +++ b/sdk/python/tests/compiler/testdata/recursive_do_while.yaml @@ -33,6 +33,9 @@ spec: image: python:alpine3.6 name: flip outputs: + artifacts: + - name: flip-output + path: /tmp/output parameters: - name: flip-output valueFrom: @@ -47,6 +50,9 @@ spec: image: python:alpine3.6 name: flip-2 outputs: + artifacts: + - name: flip-2-output + path: /tmp/output parameters: - name: flip-2-output valueFrom: @@ -61,6 +67,9 @@ spec: image: python:alpine3.6 name: flip-3 outputs: + artifacts: + - name: flip-3-output + path: /tmp/output parameters: - name: flip-3-output valueFrom: diff --git a/sdk/python/tests/compiler/testdata/recursive_while.yaml b/sdk/python/tests/compiler/testdata/recursive_while.yaml index f53bbf996ea..d7101ef1fd1 100644 --- a/sdk/python/tests/compiler/testdata/recursive_while.yaml +++ b/sdk/python/tests/compiler/testdata/recursive_while.yaml @@ -55,6 +55,9 @@ spec: image: python:alpine3.6 name: flip outputs: + artifacts: + - name: flip-output + path: /tmp/output parameters: - name: flip-output valueFrom: @@ -69,6 +72,9 @@ spec: image: python:alpine3.6 name: flip-2 outputs: + artifacts: + - name: flip-2-output + path: /tmp/output parameters: - name: flip-2-output valueFrom: @@ -83,6 +89,9 @@ spec: image: python:alpine3.6 name: flip-3 outputs: + artifacts: + - name: flip-3-output + path: /tmp/output parameters: - name: flip-3-output valueFrom: diff --git a/sdk/python/tests/compiler/testdata/sidecar.yaml b/sdk/python/tests/compiler/testdata/sidecar.yaml index 19b48aab8e5..cab18f2784f 100644 --- a/sdk/python/tests/compiler/testdata/sidecar.yaml +++ b/sdk/python/tests/compiler/testdata/sidecar.yaml @@ -23,6 +23,9 @@ spec: parameters: [] templates: - outputs: + artifacts: + - name: download-downloaded + path: "/tmp/results.txt" parameters: - name: download-downloaded valueFrom: diff --git a/sdk/python/tests/compiler/testdata/tolerations.yaml b/sdk/python/tests/compiler/testdata/tolerations.yaml index c5fe2b9c23d..4341c9f9f61 100644 --- a/sdk/python/tests/compiler/testdata/tolerations.yaml +++ b/sdk/python/tests/compiler/testdata/tolerations.yaml @@ -20,6 +20,9 @@ spec: parameters: [] templates: - outputs: + artifacts: + - name: download-downloaded + path: "/tmp/results.txt" parameters: - name: download-downloaded valueFrom: diff --git a/sdk/python/tests/compiler/testdata/volume.yaml b/sdk/python/tests/compiler/testdata/volume.yaml index b2703ccf4e2..4cd03ca7e64 100644 --- a/sdk/python/tests/compiler/testdata/volume.yaml +++ b/sdk/python/tests/compiler/testdata/volume.yaml @@ -41,6 +41,9 @@ spec: name: gcp-credentials name: download outputs: + artifacts: + - name: download-downloaded + path: /tmp/results.txt parameters: - name: download-downloaded valueFrom: diff --git a/sdk/python/tests/compiler/testdata/withparam_global.yaml b/sdk/python/tests/compiler/testdata/withparam_global.yaml index ae9c7df1a0a..b8ebf38d067 100644 --- a/sdk/python/tests/compiler/testdata/withparam_global.yaml +++ b/sdk/python/tests/compiler/testdata/withparam_global.yaml @@ -46,6 +46,9 @@ spec: image: python:alpine3.6 name: my-out-cop0 outputs: + artifacts: + - name: my-out-cop0-out + path: /tmp/out.json parameters: - name: my-out-cop0-out valueFrom: diff --git a/sdk/python/tests/compiler/testdata/withparam_global_dict.yaml b/sdk/python/tests/compiler/testdata/withparam_global_dict.yaml index a0c29186a22..e1de5e26066 100644 --- a/sdk/python/tests/compiler/testdata/withparam_global_dict.yaml +++ b/sdk/python/tests/compiler/testdata/withparam_global_dict.yaml @@ -46,6 +46,9 @@ spec: image: python:alpine3.6 name: my-out-cop0 outputs: + artifacts: + - name: my-out-cop0-out + path: /tmp/out.json parameters: - name: my-out-cop0-out valueFrom: diff --git a/sdk/python/tests/compiler/testdata/withparam_output.yaml b/sdk/python/tests/compiler/testdata/withparam_output.yaml index d9150aad37b..a3b24ac155b 100644 --- a/sdk/python/tests/compiler/testdata/withparam_output.yaml +++ b/sdk/python/tests/compiler/testdata/withparam_output.yaml @@ -43,6 +43,9 @@ spec: image: python:alpine3.6 name: my-out-cop0 outputs: + artifacts: + - name: my-out-cop0-out + path: /tmp/out.json parameters: - name: my-out-cop0-out valueFrom: diff --git a/sdk/python/tests/compiler/testdata/withparam_output_dict.yaml b/sdk/python/tests/compiler/testdata/withparam_output_dict.yaml index 9859e323e1f..44772a46a61 100644 --- a/sdk/python/tests/compiler/testdata/withparam_output_dict.yaml +++ b/sdk/python/tests/compiler/testdata/withparam_output_dict.yaml @@ -43,6 +43,9 @@ spec: image: python:alpine3.6 name: my-out-cop0 outputs: + artifacts: + - name: my-out-cop0-out + path: /tmp/out.json parameters: - name: my-out-cop0-out valueFrom: