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

Improve validation output in elyra-pipeline CLI #2070

Merged
merged 4 commits into from Aug 25, 2021
Merged

Improve validation output in elyra-pipeline CLI #2070

merged 4 commits into from Aug 25, 2021

Conversation

ptitzler
Copy link
Member

@ptitzler ptitzler commented Aug 23, 2021

What changes were proposed in this pull request?

This PR:

  • improves visualization of pipeline validation results
  • simplifies existing code to reduce maintenance efforts
  • updates pipeline validation messages for clarity (wording, punctuation, etc)
  • adds incorrect property value to validation issue payload to aid with troubleshooting
  • fixes validation bugs

The proposed output introduces a new generic message display "pattern", comprising

  • [validation-issue-severity] - always displayed
  • [node-label] - displayed if applicable and available
  • [property-name] - displayed if applicable and available
  • - <validation-message-text> - always displayed

Messages are sorted by severity (error > warning > ...), enabling the user to quickly locate issues that cause the operation to be aborted.

For clarity each component could be prefixed, but that would result in a lot more "static" output.

Example output without prefixes:

$ elyra-pipeline submit a-circle.pipeline --runtime-config my_airflow_test_instance

────────────────────────────────────────────────────────────────
 Elyra Pipeline Submission
────────────────────────────────────────────────────────────────

Validating pipeline...

[Error][i depend on you][runtime_image] - Required property value is missing. 
[Error][you depend on me][env_vars] - Property has an improperly formatted env variable key value pair. The current property value is 'what up'.
[Error][hello.ipynb][runtime_image] - Required property value is missing. 
[Error][labeled-notebook-nod][env_vars] - Property has an improperly formatted env variable key value pair. The current property value is 'humpty goes dumpty'.
[Error] - The pipeline contains a circular dependency between nodes. Nodes: 'you depend on me', 'i depend on you'
[Warning][i depend on you][label] - The node label contains characters that may be replaced by the runtime service. Node labels should start with lower case alphanumeric and contain only lower case alphanumeric, underscores, dots, and dashes. The current property value is 'i depend on you'.
[Warning][you depend on me][label] - The node label contains characters that may be replaced by the runtime service. Node labels should start with lower case alphanumeric and contain only lower case alphanumeric, underscores, dots, and dashes. The current property value is 'you depend on me'.
[Warning][hello.ipynb] - Node is not connected to any other node. 
[Warning][labeled-notebook-nod] - Node is not connected to any other node. 

Example output with prefixes:

$ elyra-pipeline submit a-circle.pipeline --runtime-config my_airflow_test_instance
────────────────────────────────────────────────────────────────
 Elyra Pipeline Submission
────────────────────────────────────────────────────────────────
Validating pipeline...

[Error][node: i depend on you][property:runtime_image] - Required property value is missing. 
[Error][node: you depend on me][property: env_vars] - Property has an improperly formatted env variable key value pair. The current property value is 'what up'.

How was this pull request tested?

Reviewed output for the following scenarios

  • validation output (no errors or warnings) run command

  • validation output (errors but no warnings) run command

  • validation output (warnings only) run command

  • validation output (no errors or warnings) submit command

  • validation output (errors but no warnings) submit command

  • validation output (warnings only) submit command

  • verify that validation changes don't break VPE output

Fixes #1999

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.

@ptitzler ptitzler added status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. component:cli-tools Command line tools labels Aug 23, 2021
@ptitzler ptitzler added this to the 3.1.0 milestone Aug 23, 2021
@elyra-bot
Copy link

elyra-bot bot commented Aug 23, 2021

Thanks for making a pull request to Elyra!

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

@ptitzler ptitzler removed the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Aug 24, 2021
@ptitzler
Copy link
Member Author

There is a known issue related to custom component validation that is not handled by this PR. For example,

$ elyra-pipeline submit custom-not-found.pipeline --runtime-config my_kubeflow_test_instance

────────────────────────────────────────────────────────────────
 Elyra Pipeline Submission
────────────────────────────────────────────────────────────────

Validating pipeline...
Traceback (most recent call last):
  File "/opt/anaconda3/envs/pr2070/bin/elyra-pipeline", line 8, in <module>
    sys.exit(pipeline())
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/elyra/cli/pipeline_app.py", line 278, in submit
    _validate_pipeline_definition(pipeline_definition)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/elyra/cli/pipeline_app.py", line 198, in _validate_pipeline_definition
    PipelineValidationManager.instance().validate(pipeline=pipeline_definition))
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete
    return future.result()
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/elyra/pipeline/validate.py", line 123, in validate
    pipeline_execution=pipeline_execution)
  File "/opt/anaconda3/envs/pr2070/lib/python3.7/site-packages/elyra/pipeline/validate.py", line 300, in _validate_node_properties
    property_dict['current_parameters'].keys()))
KeyError: 'current_parameters'

@lresende lresende merged commit 8a3816c into elyra-ai:master Aug 25, 2021
@ptitzler ptitzler deleted the improve-cli-output branch November 18, 2021 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:cli-tools Command line tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation Service warning issues has no node name set
2 participants