-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Enables uploading a pipeline via a URL #554
Enables uploading a pipeline via a URL #554
Conversation
/assign yebrahim |
Still needs verification on real cluster
1021316
to
69cfdec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one minor comment.
</div> | ||
<Input onChange={this.handleChange('fileUrl')} value={fileUrl} required={true} label='URL' /> | ||
</React.Fragment> | ||
)} | ||
|
||
<Input id='uploadFileName' label='Pipeline name' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove the upload
parts of the id and the state field here since they're also used in url import path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't disagree, but the entire component is based around that name (UploadPipelineDialog
). I'm up for renaming it in a follow-up, but what's a better verb? Add
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, probably good to handle that separately if we see the need.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test kubeflow-pipeline-e2e-test |
* Updated roadmap for 0.3 and 0.4 * more changes * blah * merged * typo
* Merge KFP changes between 1.5.0-rc.2 and 1.5.0 Resolves kubeflow#554 * Resolve backend and manifest conflicts * Resolve frontend conflicts
This change is