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 the run ID place holder to TFX sample, fix metadb config in preload sample as well #2487

Merged
merged 12 commits into from
Oct 28, 2019

Conversation

numerology
Copy link

@numerology numerology commented Oct 24, 2019

By plugging in automatically generated workflow ID the user do not need to make sure the pipeline root is clean themselves.

Included some fix to artifact store display as well.


This change is Reviewable

@numerology
Copy link
Author

/hold need to fix

@numerology
Copy link
Author

/hold need to fix

@Ark-kun
Copy link
Contributor

Ark-kun commented Oct 25, 2019

Is the binary file in this PR needed?

@numerology
Copy link
Author

Is the binary file in this PR needed?

For now, yes, we need a modified/unreleased version of TFX to compile the pipeline correctly.

@numerology
Copy link
Author

/hold cancel
Artifact problem fixed
Switch to retrieving value from envvar (there're some hidden changes in the TFX side)

@numerology
Copy link
Author

/assign @IronPan

@rmgogogo rmgogogo self-assigned this Oct 25, 2019
Copy link
Member

@IronPan IronPan left a comment

Choose a reason for hiding this comment

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

/lgtm

numerology added 2 commits October 25, 2019 08:45
# Conflicts:
#	samples/contrib/parameterized_tfx_oss/parameterized_tfx_oss.tar.gz
@k8s-ci-robot k8s-ci-robot removed the lgtm label Oct 25, 2019
@numerology
Copy link
Author

PTAL thanks @IronPan

@numerology numerology changed the title [Sample] Add the run ID place holder to TFX sample [Sample] Add the run ID place holder to TFX sample, fix metadb config in preload sample as well Oct 28, 2019
@IronPan
Copy link
Member

IronPan commented Oct 28, 2019

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IronPan

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

@IronPan
Copy link
Member

IronPan commented Oct 28, 2019

/test kubeflow-pipeline-sample-test

@k8s-ci-robot k8s-ci-robot merged commit 0ac858e into kubeflow:master Oct 28, 2019
@numerology numerology deleted the fix-tfx-sample branch October 28, 2019 19:57
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