Skip to content
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

Further big_data_passing fixes after PR #166 #194

Merged
merged 5 commits into from
Jun 24, 2020
Merged

Further big_data_passing fixes after PR #166 #194

merged 5 commits into from
Jun 24, 2020

Conversation

fenglixa
Copy link
Member

@fenglixa fenglixa commented Jun 23, 2020

Which issue is resolved by this Pull Request:
Resolves #189
Resolves #188

Description of your changes:
Based on PR #166, with this PR changes all the cases including big data should be supported now.

Environment tested:

  1. Tested all cases PASS:
(venv)  fengli@fenglis-mbp >> ~/go/src/github.com/kubeflow/ck-kfp/kfp-tekton/sdk/python/tests >>  issue_143_embedded_pipelineSpec ● >> ./run_tests.sh
test_affinity_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_basic_no_decorator (compiler.compiler_tests.TestTektonCompiler) ... ok
test_big_data_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_compose (compiler.compiler_tests.TestTektonCompiler) ... ok
test_condition_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_exit_handler_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_hidden_output_file_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_imagepullsecrets_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_init_container_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_input_artifact_raw_value_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_katib_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_loop_static_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_node_selector_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_parallel_join_with_argo_vars_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_parallel_join_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_parallel_join_workflow_with_artifacts (compiler.compiler_tests.TestTektonCompiler) ... ok
test_pipeline_transformers_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_pipelineparams_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_resourceOp_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_retry_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_sequential_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_sidecar_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_timeout_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_tolerations_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_volumeOp_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_volumeSnapshotOp_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_volume_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_withitem_nested_workflow (compiler.compiler_tests.TestTektonCompiler) ... ok
test_convert_k8s_obj_to_json_accepts_dict (compiler.k8s_helper_tests.TestK8sHelper) ... ok
test_sanitize_k8s_annotations (compiler.k8s_helper_tests.TestK8sHelper) ... ok
test_sanitize_k8s_labels (compiler.k8s_helper_tests.TestK8sHelper) ... ok
test_sanitize_k8s_name_max_length (compiler.k8s_helper_tests.TestK8sHelper) ... ok

----------------------------------------------------------------------
Ran 32 tests in 1.286s

OK
  1. Big data pipeline run successfully now:
fengli@fenglis-MacBook-Pro >> ~/go/src/github.com/kubeflow/ck-kfp/kfp-tekton >>  issue_143_embedded_pipelineSpec ● >> tkn pr describe file-passing-pipelines
Name:        file-passing-pipelines
Namespace:   tekton-pipelines

🌡️  Status

STARTED        DURATION     STATUS
1 minute ago   54 seconds   Succeeded

📦 Resources

 No resources

⚓ Params

 No params

🗂  Taskruns

 NAME                                              TASK NAME          STARTED          DURATION     STATUS
 ∙ file-passing-pipelines-print-text-5-754rt       print-text-5       49 seconds ago   15 seconds   Succeeded
 ∙ file-passing-pipelines-print-text-3-7xgm4       print-text-3       1 minute ago     15 seconds   Succeeded
 ∙ file-passing-pipelines-print-text-2-5jf6h       print-text-2       1 minute ago     19 seconds   Succeeded
 ∙ file-passing-pipelines-sum-numbers-mjhph        sum-numbers        1 minute ago     16 seconds   Succeeded
 ∙ file-passing-pipelines-print-params-nkg8g       print-params       1 minute ago     18 seconds   Succeeded
 ∙ file-passing-pipelines-print-text-4-dmckk       print-text-4       1 minute ago     16 seconds   Succeeded
 ∙ file-passing-pipelines-print-text-nvdbl         print-text         1 minute ago     16 seconds   Succeeded
 ∙ file-passing-pipelines-repeat-line-qmv42        repeat-line        1 minute ago     17 seconds   Succeeded
 ∙ file-passing-pipelines-gen-params-hr5xx         gen-params         1 minute ago     19 seconds   Succeeded
 ∙ file-passing-pipelines-split-text-lines-l8rfd   split-text-lines   1 minute ago     22 seconds   Succeeded
 ∙ file-passing-pipelines-write-numbers-rm8v5      write-numbers      1 minute ago     19 seconds   Succeeded
  1. pipeline-transformer run successfully now
(venv)  fengli@fenglis-mbp  ~/go/src/github.com/kubeflow/ck-kfp/kfp-tekton   issue_143_embedded_pipelineSpec ●  tkn pr logs -f pipeline-transformer
[print-2 : main] train my model.
[print : main] hey, what are you up to?


(venv)  fengli@fenglis-mbp  ~/go/src/github.com/kubeflow/ck-kfp/kfp-tekton   issue_143_embedded_pipelineSpec ●  tkn pr describe pipeline-transformer
Name:        pipeline-transformer
Namespace:   kubeflow

🌡️  Status

STARTED          DURATION    STATUS
17 seconds ago   9 seconds   Succeeded

📦 Resources

 No resources

⚓ Params

 No params

🗂  Taskruns

 NAME                                   TASK NAME   STARTED          DURATION    STATUS
 ∙ pipeline-transformer-print-2-kr9nx   print-2     17 seconds ago   9 seconds   Succeeded
 ∙ pipeline-transformer-print-tgb6v     print       17 seconds ago   8 seconds   Succeeded

@kubeflow-bot
Copy link

This change is Reviewable

@fenglixa
Copy link
Member Author

Travis CI failed because this PR depends on PR #166

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fenglixa -- I only have a few nit-picks :-)

sdk/python/kfp_tekton/compiler/_data_passing_rewriter.py Outdated Show resolved Hide resolved
sdk/python/kfp_tekton/compiler/_data_passing_rewriter.py Outdated Show resolved Hide resolved
sdk/python/kfp_tekton/compiler/_data_passing_rewriter.py Outdated Show resolved Hide resolved
sdk/python/kfp_tekton/compiler/_data_passing_rewriter.py Outdated Show resolved Hide resolved
@fenglixa
Copy link
Member Author

Addressed the comments, resolved the conflicts.
/cc @ckadner @Tomcli

@fenglixa fenglixa mentioned this pull request Jun 24, 2020
4 tasks
@fenglixa
Copy link
Member Author

Resolves #189

@Tomcli
Copy link
Member

Tomcli commented Jun 24, 2020

Thanks @fenglixa, do you have any prerequisite for the big_data_passing pipeline? e.g. Setting up a default pvc for workspace?

@fenglixa
Copy link
Member Author

fenglixa commented Jun 24, 2020

do you have any prerequisite for the big_data_passing pipeline? e.g. Setting up a default pvc for workspace?

Thanks @Tomcli , for your comments, it will be addressed by issue #181, which will be done by me recently. I also added the comments in the code as below:

# User need to create pvc manually recently until issue #181 addressed

@fenglixa
Copy link
Member Author

@Tomcli , I addressed your comments via sdk/README.md, please help check. Thanks!

@Tomcli
Copy link
Member

Tomcli commented Jun 24, 2020

Thanks @fenglixa
/lgtm

@fenglixa
Copy link
Member Author

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fenglixa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 9267f1f into kubeflow:master Jun 24, 2020
@fenglixa fenglixa deleted the further_big_data_passing branch June 24, 2020 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need further big_data_passing fixes after PR #166 Outstanding fixes from PR #166
5 participants