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] Add AI platform sample test #2236

Closed
wants to merge 33 commits into from

Conversation

numerology
Copy link

@numerology numerology commented Sep 25, 2019

Part of #1813

Memo: Currently using a ad hoc logic in presubmit-tests-with-pipeline-deployment.sh to GC the generated models and versions. Long term solution should be that the notebook cleans up the generated files itself, but it requires further investigation on how to grant proper permission.


This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign gaoning777
You can assign the PR to them by writing /assign @gaoning777 in a comment when ready.

The full list of commands accepted by this bot can be found 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
Copy link
Author

/test kubeflow-pipeline-sample-test

@Ark-kun
Copy link
Contributor

Ark-kun commented Sep 25, 2019

What is the benefit of renaming the samples and files?

@numerology
Copy link
Author

What is the benefit of renaming the samples and files?

Currently, the sample test infra assumes that the test file and the dir have the same name.
But I'll make a better name after making this work.

@Ark-kun
Copy link
Contributor

Ark-kun commented Sep 25, 2019

Currently, the sample test infra assumes that the test file and the dir have the same name.

Would you mind if I fix that code at some point? (I won't block your work)

@numerology
Copy link
Author

Currently, the sample test infra assumes that the test file and the dir have the same name.

Would you mind if I fix that code at some point? (I won't block your work)

Sure, that will be appreciated

@numerology
Copy link
Author

/test kubeflow-pipeline-sample-test

@numerology
Copy link
Author

/test kubeflow-pipeline-sample-test

@numerology
Copy link
Author

/retest

numerology added 4 commits October 4, 2019 14:29
…dd-mlengine-sample-test

# Conflicts:
#	test/sample_test.yaml
…dd-mlengine-sample-test

� Conflicts:
�	samples/core/ai_platform/ai_platform.ipynb
@numerology
Copy link
Author

TODO: switch back to create_run_from_pipeline_func once it works in test infra.

@numerology
Copy link
Author

/retest

@Ark-kun Ark-kun mentioned this pull request Oct 8, 2019
@numerology numerology changed the title [WIP] Add AI platform sample test Add AI platform sample test Oct 8, 2019
@numerology numerology changed the title Add AI platform sample test [Sample] Add AI platform sample test Oct 8, 2019
"arguments = {}\n",
"\n",
"# Submit a pipeline run\n",
"run_name = pipeline_func.__name__ + ' run'\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to set the run name?

Copy link
Author

Choose a reason for hiding this comment

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

Actually no. Removed.

@gaoning777
Copy link
Contributor

Looks like the service accounts are working fine. PTAL: #2327

@numerology
Copy link
Author

/close
#2327 is a better solution.

@k8s-ci-robot
Copy link
Contributor

@numerology: Closed this PR.

In response to this:

/close
#2327 is a better solution.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@numerology numerology deleted the add-mlengine-sample-test branch October 10, 2019 04:25
@numerology numerology restored the add-mlengine-sample-test branch October 10, 2019 17:01
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.

4 participants