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

DEPRECATE(sdk): DSL - Deprecated output_artifact_paths parameter in ContainerOp constructor #2334

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Oct 9, 2019

The users should switch to file_outputs instead.
Previously file_outputs only supported small data outputs, but now it supports big files.


This change is Reviewable

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 9, 2019

/retest

6 similar comments
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Oct 11, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 8, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 11, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 11, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 12, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 6, 2020

/retest

@Bobgy
Copy link
Contributor

Bobgy commented Jun 30, 2020

@Ark-kun does this need to get merged?

… constructor

The users should switch to file_outputs instead.
Previously `file_outputs` only supported small data outputs, but now it supports big files.
@Ark-kun Ark-kun force-pushed the SDK---DSL---Deprecated-output_artifact_paths-parameter-in-ContainerOp-constructor branch from f387c5a to fbf1967 Compare July 9, 2020 05:56
@Ark-kun Ark-kun force-pushed the SDK---DSL---Deprecated-output_artifact_paths-parameter-in-ContainerOp-constructor branch 2 times, most recently from f387c5a to fbf1967 Compare July 10, 2020 03:03
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 7, 2020

/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

@Bobgy Bobgy changed the title SDK - DSL - Deprecated output_artifact_paths parameter in ContainerOp constructor feat(sdk): DSL - Deprecated output_artifact_paths parameter in ContainerOp constructor Aug 7, 2020
@Bobgy Bobgy changed the title feat(sdk): DSL - Deprecated output_artifact_paths parameter in ContainerOp constructor DEPRECATE(sdk): DSL - Deprecated output_artifact_paths parameter in ContainerOp constructor Aug 7, 2020
@Bobgy
Copy link
Contributor

Bobgy commented Aug 7, 2020

What do you think about the new title?
@Ark-kun

I can add some configuration to make these PRs show up in a separate section in changelog.

@Bobgy
Copy link
Contributor

Bobgy commented Aug 7, 2020

/lgtm
/hold

leave to you to decide title

@Bobgy Bobgy added the cherrypick-approved area OWNER approves to cherry pick this PR to current active release branch label Aug 7, 2020
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 8, 2020

/unhold

@k8s-ci-robot k8s-ci-robot merged commit d87c3be into kubeflow:master Aug 8, 2020
@neuromage
Copy link
Contributor

@Ark-kun how does file outputs support big data passing? What happens if a downstream component refers to the output using inputValue?

/cc @paveldournov
/cc @hongye-sun
/cc @james-jwu

@Bobgy Bobgy removed the cherrypick-approved area OWNER approves to cherry pick this PR to current active release branch label Aug 12, 2020
Ark-kun added a commit to Ark-kun/pipelines that referenced this pull request Oct 25, 2020
Reverting most of the kubeflow#2334 which inadvernetnly broke those artifacts by causing the names to be mangled.

KFP's DSL compiler prepends template names to output names to ensure global uniqueness of *input* names (DSL's ContainerOp does not have concept of inputs, so the inputs are generated during the compilation including input names). But prepending template names to the output names stops the backend from recognizing the mlpipeline-ui-metadata and mlpipeline-metrics artifacts.
Ark-kun added a commit to Ark-kun/pipelines that referenced this pull request Oct 25, 2020
Reverting most of the kubeflow#2334 which inadvertently broke those artifacts by causing the names to be mangled.

KFP's DSL compiler prepends template names to output names to ensure global uniqueness of *input* names (DSL's ContainerOp does not have concept of inputs, so the inputs are generated during the compilation including input names). But prepending template names to the output names stops the backend from recognizing the mlpipeline-ui-metadata and mlpipeline-metrics artifacts.
k8s-ci-robot pushed a commit that referenced this pull request Oct 26, 2020
Reverting most of the #2334 which inadvertently broke those artifacts by causing the names to be mangled.

KFP's DSL compiler prepends template names to output names to ensure global uniqueness of *input* names (DSL's ContainerOp does not have concept of inputs, so the inputs are generated during the compilation including input names). But prepending template names to the output names stops the backend from recognizing the mlpipeline-ui-metadata and mlpipeline-metrics artifacts.
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
…ontainerOp constructor (kubeflow#2334)

The users should switch to file_outputs instead.
Previously `file_outputs` only supported small data outputs, but now it supports big files.
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
Reverting most of the kubeflow#2334 which inadvertently broke those artifacts by causing the names to be mangled.

KFP's DSL compiler prepends template names to output names to ensure global uniqueness of *input* names (DSL's ContainerOp does not have concept of inputs, so the inputs are generated during the compilation including input names). But prepending template names to the output names stops the backend from recognizing the mlpipeline-ui-metadata and mlpipeline-metrics artifacts.
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