Improve pipeline validation

[ajbozarth]

Upstream PR:

• Improve pipeline validation pipeline-editor#194

Includes the following improvements:

• Moves property validatiors to pipeline-services and calls them
  during node validation.
• Adds validation for keyValue string arrays
• Adds validation for pipeline properties

Validation details:

• All property validation is now run when validating the whole pipeline. This makes sure the canvas shows errors on the nodes.
• Pipeline properties are now validated during pipeline validation, usually shown when running or exporting. The validation notification has also been updated to display each validation issue on a new line in the notification.
• keyValue properties are now validated:
  • the validation simply checks if a = exists in the string and a non empty string exist on either side of it.
  • no editing is done on the string itself based on the validation

To Do:

- [ ] Don't persist auto-populated key=value strings (with no value)

Fixes #2329

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.

[elyra-bot]

Thanks for making a pull request to Elyra!

To try out this branch on binder, follow this link:

[ptitzler]

It appears that no validation is performed for the volume mount property (default and node-level) even though its input format is key/value.

[ajbozarth]

It appears that no validation is performed for the volume mount property (default and node-level) even though its input format is key/value.

IIUC that's because elyra_mounted_volumes doesn't set keyValueEntries to true to leverage the new keyValue functionality.

I can add that in this PR if you want

Edit: it's added in pipeline properties, but not node properties

[akchinSTC]

[ajbozarth]

per discussion in scrum I am adding .trim() prior to persisting strings and string arrays and am also trimming the key and value separately in the case of keyValue string arrays. I've also update the keyValue validation to allow key= where no value is set (this matches the backend which just ignores such a case).

[ajbozarth]

Also I added some screen shots with new UI elements.

I'll be updating the code sandbox version shortly with the latest changes

[akchinSTC]

Tested and working as stated.

• Pipeline defaults and node properties key list values env and volume mounts
• Pipeline does not allow submission if errors exist
• Erroneous values are persisted in pipeline on click save, this seems ok given that the pipeline will not allow submission if there is an error, but a little weird
  
• Any tests to add or update?

[marthacryan]

Frontend code changes look good - I played around a little with the UI and it looks good / seems to be working well

[ptitzler]

Looking at two possible error scenarios, Ive noticed the following behavior:

• User provides one or more invalid values as pipeline node default

  • On the canvas the problematic node is not flagged as invalid and no error is displayed when the user hovers over the node, which displays the node summary
  • In the pipeline properties view the invalid entry is flagged as expected
  • If an attempt is made to run/export the pipeline a generic error message is displayed indicating that a property value is invalid, without giving an indication where the property is defined (pipeline default vs node)

  

• User provides one or more invalid node value

  • On the canvas the problematic node is flagged as invalid and an error is displayed when the user hovers over the node, which displays the node summary
  • In the pipeline properties view the invalid entry is flagged as expected
  • If an attempt is made to run/export the pipeline a generic error message is displayed indicating that a property value is invalid, without giving an indication where the property is defined (pipeline default vs node)

From a user's point of view it would be easier to troubleshoot the issue if the VPE would expose where the problem was detected.

[ajbozarth]

A few comments on the above cases:

without giving an indication where the property is defined (pipeline default vs node)

There is an indication, each individual error message is printed on a separate line and either says it's a pipeline property on what node its on in the message. It may be worth making the error message timeout longer though (or removing the time out so it has to be manually cleared) since it seems it doesn't persist long enough for you to notice that.

To deal with that I can increase the timeout from 6s to 30s and add close on clickAway functionality to the error message

[ajbozarth]

To deal with that I can increase the timeout from 6s to 30s and add close on clickAway functionality to the error message

just pushed this update

[ajbozarth]

Just released pipeline-editor v1.9.0 with the upstream changes. If everyone could do a final check on this

[ptitzler]

LGTM