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

De-hardcode output paths in all components #4100

Closed
3 of 4 tasks
Ark-kun opened this issue Jun 28, 2020 · 8 comments
Closed
3 of 4 tasks

De-hardcode output paths in all components #4100

Ark-kun opened this issue Jun 28, 2020 · 8 comments
Assignees
Labels
area/components help wanted The community is welcome to contribute. lifecycle/frozen status/triaged Whether the issue has been explicitly triaged

Comments

@Ark-kun
Copy link
Contributor

Ark-kun commented Jun 28, 2020

Some components are using the deprecated fileOutputs attribute. See the query

The fileOutputs was deprecated from the start and should not be used in any non-legacy components.

We need to migrate from the deprecated fileOutputs to use the correct {outputPath: output_name} command-line placeholders instead.

Example:

Before:

implementation:
  container:
    command:
    - my_prog.py
    - --param1
    - value1
    fileOutputs:
      some_output: /some/hardcoded/path

After:

implementation:
  container:
    command:
    - my_prog.py
    - --param1
    - value1
    - --some-output
    - {outputPath: some_output}

The component code should be modified to receive all output paths through command-line arguments instead of hard-codong any paths.

An example of the correction: https://github.com/kubeflow/pipelines/pull/580/files#diff-9c31c4b41f436d2bbdfc9c97d40b9fe1
https://github.com/kubeflow/pipelines/pull/580/files#diff-ec0c67e038e3087e2bb01d439771bddc

There are two main groups of affected components:

@Bobgy
Copy link
Contributor

Bobgy commented Jul 3, 2020

@Ark-kun is this 1.0 release blocking?

Shall we add it to release notice? what kind of error message will people get if using this file_output?

@NikeNano
Copy link
Member

NikeNano commented Jul 6, 2020

@JohanWork

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Jul 7, 2020

fileOutputs was deprecated from the start. It was added to accommodate legacy containers that had hardcoded paths. No new components were supposed to use it.
I'll add a formal deprecation message: "The fileOutputs map is deprecated. Please use {outputPath: output_name} command-line placeholders instead.".
It won't be a hard error.

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Jul 7, 2020

is this 1.0 release blocking?

I do not think so. The components are not part of the release.

@stale
Copy link

stale bot commented Dec 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Dec 18, 2020
@Bobgy
Copy link
Contributor

Bobgy commented Dec 22, 2020

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen and removed lifecycle/stale The issue / pull request is stale, any activities remove this label. labels Dec 22, 2020
@RedbackThomson RedbackThomson removed their assignment Feb 3, 2022
@rimolive
Copy link
Member

rimolive commented Mar 7, 2024

Looks like this work is done. Closing this issue.

/close

Copy link

@rimolive: Closing this issue.

In response to this:

Looks like this work is done. Closing this issue.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/components help wanted The community is welcome to contribute. lifecycle/frozen status/triaged Whether the issue has been explicitly triaged
Projects
Status: Closed
Development

No branches or pull requests

8 participants