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

Sample test improvement - using python fire to launch sample test #1897

Merged
merged 81 commits into from
Aug 23, 2019

Conversation

numerology
Copy link

@numerology numerology commented Aug 20, 2019

Switch from bash script to python fire fore better manageability.

Example command line usage:
(Sample test run): python sample_test_launcher.py sample_test run_test --arg1 value1 --arg2 value2
(Sample test run): python sample_test_launcher.py component_test run_test --arg1 value1 --arg2 value2

Component test is defined as a derived class of sample test to reduce dup logic.
A step of #1750


This change is Reviewable

numerology and others added 30 commits July 23, 2019 15:13
commit 7dbca1a
Author: Ning <ngao@google.com>
Date:   Fri Aug 16 12:06:37 2019 -0700

    update gcloud ml-engine to ai-platform (kubeflow#1863)

commit e849f22
Author: Ryan Dawson <ryandawson@cantab.net>
Date:   Fri Aug 16 19:21:23 2019 +0100

    Seldon examples (kubeflow#1405)

commit a6f3f40
Author: sina chavoshi <sina.chavoshi@gmail.com>
Date:   Fri Aug 16 09:56:09 2019 -0700

    Adding a sample for serving component (kubeflow#1830)

    * Adding a sample for serving component

    * removed typo / updated based on PR feedback

    * fixing the jupyter rendering issue

    * adding pip3 for tensorflow

    * Fixed spelling error in VERSION

    * fix indentation based on review feedback

commit 54ff3e6
Author: Alexey Volkov <avolkov@google.com>
Date:   Fri Aug 16 01:22:31 2019 -0700

     SDK - Cleanup - Serialized PipelineParamTuple does not need value or type (kubeflow#1469)

    * SDK - Refactoring - Serialized PipelineParam does not need type
    Only the types in non-serialized PipelineParams are ever used.

    * SDK - Refactoring - Serialized PipelineParam does not need value
    Default values are only relevant when PipelineParam is used in the pipeline function signature and even in this case compiler captures them explicitly from the pipelineParam objects in the signature.
    There is no other uses for them.

commit d2e94e4
Author: Riley Bauer <34456002+rileyjbauer@users.noreply.github.com>
Date:   Thu Aug 15 19:52:34 2019 -0700

    Fix run duration bug (kubeflow#1827)

    * Allows durations >=24h and renames 'showLink' in RunList

    * Update, fix tests

commit d8eaeaa
Author: Alexey Volkov <avolkov@google.com>
Date:   Thu Aug 15 17:25:59 2019 -0700

    SDK - Preserving the pipeline input information in the compiled Workflow (kubeflow#1381)

    * SDK - Preserving the pipeline metadata in the compiled Workflow

    * Stabilizing the DSL compiler tests

commit afe8a69
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 15 17:00:28 2019 -0700

    Reduce API usage by utilizing reference name in reference resource API (kubeflow#1824)

    * Regenerated run api for frontend

    * Added support for reference name to resource reference API in frontend

    * Revert "Regenerated run api for frontend"

    * Addressed PR comments

    * Removed extra if statement by setting default value of parameter

    * Removed the whole comment

    * Addressed PR feedback

    * Addressed PR feedback

    * Simplified logic after offline discussion

commit ea67c99
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 15 16:18:35 2019 -0700

    Change how Variables are Provided to Visualizations (kubeflow#1754)

    * Changed way visualization variables are passed from request to NotebookNode

    Visualization variables are now saved to a json file and loaded by a NotebookNode upon execution.

    * Updated roc_curve visualization to reflect changes made to dependency injection

    * Fixed bug where checking if is_generated is provided to roc_curve visualization would crash visualizaiton

    Also changed ' -> "

    * Changed text_exporter to always sort variables by key for testing

    * Addressed PR suggestions

commit d238bef
Author: Jiaxiao Zheng <jxzheng@google.com>
Date:   Thu Aug 15 12:55:29 2019 -0700

    Add back coveralls. (kubeflow#1849)

    * Remove redundant import.

    * Simplify sample_test.yaml by using withItem syntax.

    * Simplify sample_test.yaml by using withItem syntax.

    * Change dict to str in withItems.

    * Add back coveralls.

commit 0517114
Author: Riley Bauer <34456002+rileyjbauer@users.noreply.github.com>
Date:   Thu Aug 15 12:28:35 2019 -0700

    Reduce getPipeline calls in RunList (kubeflow#1852)

    * Skips calling getPipeline in RunList if the pipeline name is in the pipeline_spec

    * Update fixed data to include pipeline names in pipeline specs

    * Remove redundant getRuns call

commit 39e5840
Author: IronPan <yangpa@google.com>
Date:   Thu Aug 15 11:04:34 2019 -0700

    Add retry button in Pipeline UI (kubeflow#1782)

    * add retry button

    * add retry button

    * add retry button

    * address comments

    * fix tests

    * fix tests

    * update image

    * Update StatusUtils.test.tsx

    * Update RunDetails.test.tsx

    * Update Buttons.ts

    * update test

    * update frontend

    * update

    * update

    * addrerss comments

    * update test
step 1, presubmit check triggered by fire.
step 2, add ComponentTest class
Rewrite dockerfile
Merge branch 'master' of https://github.com/kubeflow/pipelines into test-improvement

# Conflicts:
#	sdk/python/tests/compiler/testdata/retry.yaml
commit 0ed5819
Author: Yuan (Bob) Gong <gongyuan94@gmail.com>
Date:   Tue Aug 20 08:13:31 2019 +0800

    Refactor presubmit-tests-with-pipeline-deployment.sh to run in other projects (kubeflow#1732)

    * Refactor presubmit-tests-with-pipeline-deployment.sh so that it can be run from a different project

    * Simplify getting service account from cluster.

    * Copy image builder image instead of granting permission

    * Add missed yes command

    * fix stuff

    * Let other usages of image-builder image become configurable

    * let test workflow use image builder image
numerology added 2 commits August 22, 2019 19:34
commit 7b6825d
Author: hongye-sun <43763191+hongye-sun@users.noreply.github.com>
Date:   Thu Aug 22 18:23:18 2019 -0700

    Update changelog for 0.1.27 (kubeflow#1935)

    * Update changelog for 0.1.27

    * Revert unexpected changes.

commit 336760c
Author: IronPan <yangpa@google.com>
Date:   Thu Aug 22 17:47:18 2019 -0700

    sync namespaced install file (kubeflow#1932)

commit 55d62fe
Author: Hamed <hamedhsn@gmail.com>
Date:   Fri Aug 23 01:09:18 2019 +0100

    Support Affinity for ContainerOps (kubeflow#1886)

commit d15697b
Author: Animesh Singh <singhan@us.ibm.com>
Date:   Thu Aug 22 16:29:17 2019 -0700

    Initial kfserving pipeline component (kubeflow#1838)

    * initial kfserving pipeline component

    * changing some descriptions

    * adding action as an input, and pinning versions

    * moving Dockerfile in correct location

    * adding namespace to delete

commit 119405d
Author: hongye-sun <43763191+hongye-sun@users.noreply.github.com>
Date:   Thu Aug 22 15:49:29 2019 -0700

    upgrade backend image versions (kubeflow#1918)

commit c01315a
Author: Alexey Volkov <avolkov@google.com>
Date:   Thu Aug 22 15:31:24 2019 -0700

    SDK - Refactoring - Replaced the TypeMeta class (kubeflow#1930)

    * SDK - Refactoring - Replaced the TypeMeta class
    The PipelineParam no longer exposes the private TypeMeta class
    Fixes kubeflow#1420

    The refactoring PR is part of a series of PR which unifies the metadata and specification types.

commit 736c7a5
Author: IronPan <yangpa@google.com>
Date:   Thu Aug 22 15:29:19 2019 -0700

    clean up owner file (kubeflow#1928)

commit 5abba66
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 22 14:45:18 2019 -0700

    Add pipeline id to pipeline summary card (kubeflow#1927)

    * Added pipeline id to pipeline summary card

    * Updated PipelineDetails.test.tsx.snap

commit 3c8952e
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 22 13:57:20 2019 -0700

    Add TFDV, TFMA, and Table visualization support for Python based visualizations (kubeflow#1898)

    * Added table and tfdv visualization

    Also fixed issue surrounding ApiVisualizationType enum

    * Fixed table visualization

    * Removed byte limit
    * Fixed issue where headers would not properly be applied
    * Fixed issue where table would not be intractable

    * Updated table visualizaiton to reflect changes made to dependency injection

    * Fixed bug where checking if headers is provided to table visualizations could crash visualization

    * Added TFMA visualization

    * Updated new visualizations to match syntax of kubeflow#1878

    * Updated test snapshots to account for TFMA visualization

    * Small if statement synax changes

    * Add flake8 noqa comments to table.py and tfma.py

commit db6c9b4
Author: dushyanthsc <43390008+dushyanthsc@users.noreply.github.com>
Date:   Thu Aug 22 13:11:18 2019 -0700

    pipeline-lite: Introduce metadata component to pipeline-lite (kubeflow#1840)

    This change introduces the metadata component to pipeline-lite
    installation. This installation:

      1. Does not include metadata-ui
      2. mysql installation is used instead of metadata-db
      3. Replica count has been reduced 1 instead of 3
blob.upload_from_filename(source_file_name)
except:
raise RuntimeError('Failure when uploading %s from %s to %s'
Copy link
Contributor

Choose a reason for hiding this comment

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

The errors that GCS python throws might still help. For example, get_bucket throws bucket_not_found exception. What do you think of outputting this message before re-throwing the caught exception?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

except:
raise RuntimeError('Failure when uploading %s from %s to %s'
% (source_file_name, bucket_name, destination_blob_name))
except google.cloud.exceptions.GoogleCloudError as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, except's can be combined as such:
try:
except A:
except B:

Copy link
Author

Choose a reason for hiding this comment

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

Good call. Done.

@gaoning777
Copy link
Contributor

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

@numerology numerology merged commit 5553566 into kubeflow:master Aug 23, 2019
@Bobgy
Copy link
Contributor

Bobgy commented Aug 26, 2019

Hi @numerology , this PR seems to break postsubmit test. Would you mind double check what went wrong there?
https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/kubeflow-pipeline-postsubmit/1165032256935301120#0:build-log.txt%3A3214

run-tfx-tests:	Run the sample tests...
run-tfx-tests:	Traceback (most recent call last):
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 256, in <module>
run-tfx-tests:	    main()
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 252, in main
run-tfx-tests:	    'component_test': ComponentTest
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 138, in Fire
run-tfx-tests:	    component_trace = _Fire(component, args, parsed_flag_args, context, name)
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 471, in _Fire
run-tfx-tests:	    target=component.__name__)
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 675, in _CallAndUpdateTrace
run-tfx-tests:	    component = fn(*varargs, **kwargs)
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 238, in run_test
run-tfx-tests:	    self._injection()
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 232, in _injection
run-tfx-tests:	    subs)
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/utils.py", line 84, in file_injection
run-tfx-tests:	    tmp_line = re.sub(regex, new)
run-tfx-tests:	TypeError: sub() missing 1 required positional argument: 'string'

@numerology
Copy link
Author

Hi @numerology , this PR seems to break postsubmit test. Would you mind double check what went wrong there?
https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/kubeflow-pipeline-postsubmit/1165032256935301120#0:build-log.txt%3A3214

run-tfx-tests:	Run the sample tests...
run-tfx-tests:	Traceback (most recent call last):
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 256, in <module>
run-tfx-tests:	    main()
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 252, in main
run-tfx-tests:	    'component_test': ComponentTest
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 138, in Fire
run-tfx-tests:	    component_trace = _Fire(component, args, parsed_flag_args, context, name)
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 471, in _Fire
run-tfx-tests:	    target=component.__name__)
run-tfx-tests:	  File "/usr/local/lib/python3.5/dist-packages/fire/core.py", line 675, in _CallAndUpdateTrace
run-tfx-tests:	    component = fn(*varargs, **kwargs)
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 238, in run_test
run-tfx-tests:	    self._injection()
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/sample_test_launcher.py", line 232, in _injection
run-tfx-tests:	    subs)
run-tfx-tests:	  File "/python/src/github.com/kubeflow/pipelines/test/sample-test/utils.py", line 84, in file_injection
run-tfx-tests:	    tmp_line = re.sub(regex, new)
run-tfx-tests:	TypeError: sub() missing 1 required positional argument: 'string'

NP. I'll do a quick fix. Sorry for the inconvenience.

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.

5 participants