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
add the form flag for deploying with serverless #2020
add the form flag for deploying with serverless #2020
Conversation
frontend/packages/dev-console/src/components/import/import-types.ts
Outdated
Show resolved
Hide resolved
frontend/packages/dev-console/src/components/import/serverless/ServerlessSection.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/dev-console/src/components/import/DeployImage.tsx
Outdated
Show resolved
Hide resolved
8d2f77e
to
72cc3c2
Compare
// actions.setSubmitting(false); | ||
// actions.setStatus({ submitError: err.message }); | ||
// }); | ||
// } |
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.
@christianvogt you asked for this to be moved on the other PR that copies this one. So I moved it, but I don't see the value.
I left it commented out so you could see what was moved and to turn it back on as needed.
The serverless function does not support the dryness test so they need to be separated. This also takes the action
. I think it makes sense to keep it here but I've moved it to see if this is really what you want.
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.
I thought dry run was for any resource?
Regardless, I was not asking to move the entire function, but rather the if conditions related to if serverless or not into the util so that the Component only needs to deal with whether the creation failed or succeeded and then perform the necessary actions. The util would be responsible for creating the resources whether they be knative or not.
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.
I think when I first put this together I had issues with the dry run and so had to move it out. That is the reason it is separated like you see here.
I'll take another look and confirm it.
If it can't work with dry run then I'm thinking to put it all back the way it was.
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.
dry run
doesn't seem to work with Knative, it returns 5XX
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.
I worked out how to move it to the submit utils.
frontend/packages/dev-console/src/components/import/serverless/ServerlessSection.tsx
Outdated
Show resolved
Hide resolved
72cc3c2
to
65e0f15
Compare
/retest |
65e0f15
to
2615c4f
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
/assign @benjaminapetersen |
if (flags[FLAG_KNATIVE_SERVING]) { | ||
return ( | ||
<FormSection title="Serverless Options" divider> | ||
<div className="pf-c-button odc-serverless-tech-preview">Tech Preview</div> |
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.
I think you should be using PF4 label component here.
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.
I updated it based on direction from @serenamarie125 to use
<span className="label label-warning">Tech Preview</span>
2615c4f
to
b1c5990
Compare
b1c5990
to
7659dee
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, invincibleJai, joshuawilson 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 |
/retest |
This will add the Serverless checkbox to the Deploy Image form.