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.

Sign up for GitHub

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

Jump to bottom

SDK - Client - Added a way to set experiment name using environment variables #2292

Merged
merged 9 commits into from
Oct 14, 2019
Merged

SDK - Client - Added a way to set experiment name using environment variables #2292

merged 9 commits into from
Oct 14, 2019

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Oct 2, 2019 •
edited
Loading

This is useful for controlling the experiment name when launching notebooks or pipeline files that submit themselves for execution.

This change is Reviewable

@Ark-kun Ark-kun requested a review from numerology October 2, 2019 23:16
@numerology
Copy link

Thank you for doing this @Ark-kun !

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 3, 2019

/retest

2 similar comments
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 5, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 7, 2019

/retest

@Ark-kun Ark-kun force-pushed the SDK---Client---Added-a-way-to-set-experiment-name-using-environment-variables branch from a49ec53 to b756625 Compare October 7, 2019 22:57
@Ark-kun Ark-kun mentioned this pull request Oct 10, 2019
Closed
@numerology numerology mentioned this pull request Oct 10, 2019
Closed
@Ark-kun Ark-kun force-pushed the SDK---Client---Added-a-way-to-set-experiment-name-using-environment-variables branch 2 times, most recently from 65cc605 to 9f1fe08 Compare October 11, 2019 02:22
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 11, 2019

/retest

@Ark-kun Ark-kun force-pushed the SDK---Client---Added-a-way-to-set-experiment-name-using-environment-variables branch from 83eba82 to 6e26a84 Compare October 11, 2019 03:51
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 11, 2019

/test kubeflow-pipeline-sample-test

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 11, 2019

/retest

@Ark-kun Ark-kun force-pushed the SDK---Client---Added-a-way-to-set-experiment-name-using-environment-variables branch from 5059495 to b22cc3e Compare October 12, 2019 01:40
@Ark-kun
Changed the SDK installation lines in samples
e381192 
Otherwise the sample tests do not correctly test the new SDK code.
@Ark-kun Ark-kun force-pushed the SDK---Client---Added-a-way-to-set-experiment-name-using-environment-variables branch from b22cc3e to e381192 Compare October 12, 2019 02:52
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 12, 2019

/cc @numerology This PR is now fully ready.

@gaoning777
Copy link
Contributor

What is the default experiment name when the env_var is not set?

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 14, 2019

What is the default experiment name when the env_var is not set?

Back in spring, KFP has introduced the concept of default experiment.
When the experiment_name is not specified in create_run_from_pipeline_func, if will use that default experiment ("Default"). This PR does not change that.
See https://github.com/kubeflow/pipelines/pull/2292/files#diff-1f65a44a2187458da0f00e0ebbe730e5L368 and https://github.com/kubeflow/pipelines/pull/2292/files#diff-1f65a44a2187458da0f00e0ebbe730e5R375

@numerology
Copy link

/lgtm

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 14, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ark-kun

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 7b6957a into kubeflow:master Oct 14, 2019
@numerology numerology mentioned this pull request Oct 18, 2019
Closed
@Ark-kun Ark-kun mentioned this pull request Dec 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@numerology numerology numerology left review comments

@hongye-sun hongye-sun Awaiting requested review from hongye-sun

Assignees

@numerology numerology

Labels
approved area/sdk/client area/testing lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ark-kun @numerology @gaoning777 @k8s-ci-robot