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

Update notebook node to allow multiple input links #759

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

vabarbosa
Copy link
Contributor

@vabarbosa vabarbosa commented Jul 15, 2020

Fixes #750

This addresses the issue of cardinality incorrectly being set to a max: 0 and thus not allowing multiple input links on the notebook node.

The second observation(cardinality being deleted after first node) was addressed in canvas (elyra-ai/canvas#162) and should be part of the next canvas release.

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@vabarbosa vabarbosa self-assigned this Jul 15, 2020
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

LGTM - thanks Va!

@lresende
Copy link
Member

Do we know what happens if we don't specify cardinality info for the nodes? Are there an acceptable default?

@ajbozarth
Copy link
Member

As far as we've seen having no cardinality behaves the same as we would like it to

@kevin-bates
Copy link
Member

Although we see a behavior of min: 0, max: -1, the schema imposes an "at least 1" default while the comment in the description indicates an "exactly 1" (min: 1, max: 1) behavior for inputs and an "at least 1" (min: 1, max: -1) behavior for outputs. So they might have a bug when cardinality is not specified (IMO).

All the more reason to update the canvas for 1.0.

@lresende
Copy link
Member

The answer to my original question:

The original intent for palettes was to load selected operators from an operator repository. The operator definition includes “cardinality” for the “port_definition”: https://github.com/elyra-ai/pipeline-schemas/blob/master/common-pipeline/operators/operator-v3-schema.json.
The description for “cardinality” is:
“Property to capture how many connections are allowed for this port, e.g., min: 1, max:1 implies you must supply 1 and only 1; min:0, max:1 implies it is optional and a max of one, min:0, max:-1 means it is optional and you can have any number of connections. The default value is 1:1 for inputs and 1:-1 for outputs.”

@kevin-bates
Copy link
Member

Sounds like what I said and referenced. I should win a prize: 🎉 😄

For ports with cardinality min > 0, when does that get enforced? Is there some call to make to "validate a pipeline" prior to its submission?

@lresende lresende merged commit 6031b07 into elyra-ai:master Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pipeline editor prevents multiple input links on first node added after open
4 participants