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

Nested workflow input declarations should indicate whether they are optional or required #1059

Open
glopesdev opened this issue Sep 23, 2022 · 1 comment
Labels
bug Something isn't working proposal Request for a new feature
Milestone

Comments

@glopesdev
Copy link
Member

The current error message when specifying nested nodes with multiple inputs can be confusing. The current behavior is to let the compiler evaluate whether the extra inputs are needed or not, which is great for situations allowing for dynamic optional inputs, but very confusing if the inputs are actually required.

The main source of confusion is that the outer nested node might report an error in the number of arguments to an inner node which can be completely consistent with the number of inputs which are given to the outer node. This then requires delving into the operator definition to understand why exactly the error is happening, when in reality it can be a simple mistake in the supplied number of arguments.

@glopesdev glopesdev added the bug Something isn't working label Sep 23, 2022
@glopesdev
Copy link
Member Author

For a more explicit example of this problem, consider the following group workflow:

image

The first input is routed directly to the output, the second is sent to a local transform, and the third one is ignored.

image

When calling this group workflow with a single input, we will receive the following misleading error message at the top-level: Unsupported number of arguments. This node requires at least 1 input connection(s).

This error is thrown by the inner MemberSelector node, which does indeed require at least 1 input but is not receiving any since the 2nd input to the group was not provided. However, the reason this is misleading to read at the top-level is that we are sending one input to the group so technically the message at that level is not correct.

In the past we did enforce that all WorkflowInput arguments were strictly required, and therefore the group would be able to throw early if not enough inputs are provided.

As mentioned in the issue, it might be better to revert to the previous semantics and make a decision that when we do support optional types we will have to provide syntax to explicitly label an input as "optional" and operators to explicitly allow manipulating connections depending on whether the input is provided.

As it currently stands, the support for "optional" types is in practice extremely limited since there is really no good support for handling optional arguments inside the workflow itself.

@glopesdev glopesdev added the proposal Request for a new feature label Nov 30, 2024
@glopesdev glopesdev added this to the 2.9 milestone Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working proposal Request for a new feature
Projects
None yet
Development

No branches or pull requests

1 participant