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

Prevent run from starting if it has missing parameters #1487

Closed
ajchili opened this issue Jun 11, 2019 · 2 comments
Closed

Prevent run from starting if it has missing parameters #1487

ajchili opened this issue Jun 11, 2019 · 2 comments

Comments

@ajchili
Copy link
Member

ajchili commented Jun 11, 2019

Currently, a run is able to be executed without parameters. For instance, if you wanted to use [Sample] ML - TFX - Taxi Tip Prediction Model Trainer, you have to specify the output location and project name while all other parameters are provided. For a new comer to KFP or someone who is rushing through the run process, these fields could be overlooked and ignored. This becomes problematic as these fields can be left empty and the start button is still clickable. This results in a failed run as the project and output location are missing. Looking at the NewRun.tsx file, there is a portion of code that maps parameters to TextFields. Something similar can be used here to disable the button should a parameter be missing its value. An approach for solving this issue is below (which I would be happy to implement given the time 😀).

...
private areParametersMissing(): boolean {
  if (pipeline && Array.isArray(pipeline.parameters) && !!pipeline.parameters.length) {
    let missingParameters = pipeline.parameters.filter(parameter => !!parameter.value);
    return !!missingParameters.length;
  }
  // Returns true by default since a pipeline is required to start a run
  return true;
};
...
render() {
...
      <BusyButton id='startNewRunBtn'
        disabled={!!errorMessage || this._areParametersMissing()}
        busy={this.state.isBeingStarted}
        className={commonCss.buttonAction} title='Start'
        onClick={this._start.bind(this)} />
...
}

Alternatively, should there be a need for flexibility where blank parameters are needed, a warning can be displayed via a dialog to the user before confirming a run.

@ajchili
Copy link
Member Author

ajchili commented Oct 2, 2019

/close
Fixed with #2016

@k8s-ci-robot
Copy link
Contributor

@ajchili: Closing this issue.

In response to this:

/close
Fixed with #2016

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants