Add notebook sample test: tfx sample #470
Conversation
…, fix minor bugs add CMLE model deletion
…r model name and version
|
solving #263 |
gaoning777
changed the title
| "client = kfp.Client()\n", | ||
| "exp = client.create_experiment(name='demo')" | ||
| "exp = client.create_experiment(name=EXPERIMENT_NAME)" |
There was a problem hiding this comment.
I think we should reuse the experiment if it already exists.
There was a problem hiding this comment.
Yes. But for that, we can have another PR to do that. The scope for this PR is to add the sample tests, and it requires parameterizing the pipeline notebooks.
| "EVAL_DATA = 'gs://ml-pipeline-playground/tfx/taxi-cab-classification/eval.csv'\n", | ||
| "HIDDEN_LAYER_SIZE = '1500'\n", | ||
| "STEPS = 3000\n", | ||
| "DATAFLOW_TFDV_IMAGE = 'gcr.io/ml-pipeline/ml-pipeline-dataflow-tfdv:0.1.3-rc.2'#TODO-release: update the release tag for the next release\n", |
There was a problem hiding this comment.
Taking the images out of the components does not look great.
Can we do the following: The notebook compiles the pipeline as normal, but then the image buckets and tags are replaced. Similarly to how the sample tests are performed.
| @@ -236,13 +250,7 @@ | |||
| "#Get or create an experiment and submit a pipeline run\n", | |||
| "import kfp\n", | |||
| "client = kfp.Client()\n", | |||
| "list_experiments_response = client.list_experiments()\n", | |||
There was a problem hiding this comment.
Why is this code being removed?
There was a problem hiding this comment.
The test needs to know the experiment name in order to check the status for test results.
|
I'm approving it, but we should put the images back inside the component definitions once we have better image substitution facilities: #487 |
|
/lgtm |
|
I've looked into the pieces that touches the test workflow and permission. didnt looked into the test logic. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 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 |
|
/lgtm cancel Waiting for review from @qimingj since the notebook was created by him. |
|
/lgtm Awesome! |
* add notebook sample tests for tfx * parameterize component image tag * parameterize base and target image tags * install tensorflow package for the notebook tfx sample test * bug fixes * start debug mode * fix bugs * add namespace arg to check_notebook_results, copy test results to gcs, fix minor bugs add CMLE model deletion * install the correct KFP version in the notebook; parameterize deployer model name and version * fix CMLE model name bug * add notebook sample test in v2 * add gcp sa in notebook tfx sample and shutdown debug mode * import kfp.gcp
* Fix parallel_join_with_argo_vars pipeline Resolves kubeflow#470 * Fix broken doc links for IKS install
This change is