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

sdk: add dsl.OneOf docs #3605

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

connor-mccarthy
Copy link
Member

No description provided.

@connor-mccarthy
Copy link
Member Author

/hold
Do not merge until after kubeflow/pipelines#10067 is released

@connor-mccarthy
Copy link
Member Author

/remove-approve

@connor-mccarthy
Copy link
Member Author

/assign @chensun


You should provide task outputs to the `dsl.OneOf` using `.output` or `.outputs[<key>]`, just as you would pass an output to a downstream task. The outputs provided to `dsl.OneOf` must be of the same type and cannot be other instances of `dsl.OneOf` or [`dsl.Collected`][dsl-collected].

{{% oss-be-unsupported feature_name="`dsl.OneOf`" %}}
Copy link
Member

Choose a reason for hiding this comment

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

We should be consistent on where to put this note.
For features listed in this page, dsl.OneOf has it at the bottom, dsl.ParallelFor has it in the middle, and dsl.Collected has it at the top.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that. Moved them all to the bottom.

Reason for bottom: One of the notices is about the parallelism setting on dsl.ParallelFor, which doesn't make sense to put at the top before the parallelism setting has been introduced. Between putting at the bottom and at various places in the middle, I prefer putting everything at the bottom since it invites more uniformity.

Copy link
Member

@chensun chensun left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 18, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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

@connor-mccarthy
Copy link
Member Author

/unhold

@google-oss-prow google-oss-prow bot merged commit f697081 into kubeflow:master Oct 27, 2023
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.

2 participants