elyra-ai / elyra

Elyra extends JupyterLab with an AI centric approach.
https://elyra.readthedocs.io/en/stable/
Apache License 2.0
1.86k stars 343 forks source link

Validator wrongly doesn't validate parent property #3019

Open romeokienzler opened 2 years ago

romeokienzler commented 2 years ago

Describe the issue When selecting a parent output as input in the properties sidebar of a kfp component the validator wrongly doesn't validate

To Reproduce Steps to reproduce the behavior:

  1. clone repo at given commit and examine this pipeline

Screenshots or log output image

Expected behavior Pipeline validates

Deployment information Describe what you've deployed and how:

ptitzler commented 2 years ago

I was able to recreate the issue. The component specification declares data_json to be passed as inputValue instead of inputPath (third line from the bottom):

name: spark-json-to-parquet
description: Converts a JSON file to parquet using ApacheSpark CLAIMED v0.2m

inputs:
- {name: data_json, type: String, description: 'source path and file name (default: data.csv)'}
- {name: master, type: String, description: 'url of master (default: local mode)'}
- {name: data_dir, type: String, description: 'temporal data storage for local execution'}

outputs:
- {name: output_data_parquet, type: String, description: 'destination path and parquet file name (default: data.parquet)'}

implementation:
    container:
        image: romeokienzler/claimed-spark-json-to-parquet:0.2m
        command:
        - sh
        - -ec
        - |
          ipython ./spark-json-to-parquet.ipynb output_data_parquet="$0" data_json="$1" master="$2" data_dir="$3" 
        - {outputPath: output_data_parquet}
        - {inputValue: data_json}
        - {inputValue: master}
        - {inputValue: data_dir}

@kiersten-stokes @ajbozarth what are your thoughts? Was our intention to have a constraint that one can only select an upstream node output as input if the passing mode is inputPath? Irrespective of what our intention was, the current behavior appears to be a bug.

kiersten-stokes commented 2 years ago

@kiersten-stokes @ajbozarth what are your thoughts? Was our intention to have a constraint that one can only select an upstream node output as input if the passing mode is inputPath? Irrespective of what our intention was, the current behavior appears to be a bug.

Yes, that is intentional behavior. It would be fairly easy to remove that constraint on the backend.

ptitzler commented 2 years ago

It would be fairly easy to remove that constraint on the backend.

Great. In that case let's do that then. As is users would have to change their component definition and implementation to exchange data with other components.