-
Notifications
You must be signed in to change notification settings - Fork 3
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
[ui] Supply service chart values when creating a service #300
Conversation
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
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.
in the shared epinio i've updated the mysql-dev
service to include
"ingress.enabled":
type: "bool"
"ingress.hostname":
type: "string"
- clicking the ingress.enabled checkbox doesn't check the box until text is entered in the
ingress.hostname
field - when saving this the service failed to be created (could be that it expects the settings to correlate with helm variables?)
- Edit - this did eventually deploy. we might not be waiting long enough for the deployed state
- the detail view (read only edit view) shows quotes for the text
Note https://deploy-preview-265--epinio-docs-staging.netlify.app/next/howtos/using_custom_service_values Note especially the array and map support. |
I could verify this bug on Service charts, applications chart work fine. Needs investigation
This is also happening testing The UI correctly sends string values without double quotes, then epinio returns back settings with quoted strings: It seems to be a regression, needs to double check with the be team and open an issue, in case. |
I'm opening a new issue to support new array types: #302 . |
Thanks @richard-cox , small issues from UI side are fixed. There are a few related pending issues :
|
Signed-off-by: Francesco Torchia <[email protected]>
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.
there's some interesting things going on with the spec (added a comment to #302 (comment)). those don't block this pr though
}; | ||
}, | ||
|
||
mounted() { | ||
if (this.mode !== 'create') { | ||
this.value.details().then((details: any) => { |
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.
See comment for service model details
.
This will make a http request to fetch the service that we're already viewing. If the user refreshes on the page it'll be the second request to fetch the same service.
If there are scenarios where the settings
section is missing we should make the request optional given that scenario (and use epinio/find with force: true when fetching).
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.
Replaced with store
function.
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.
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.
Additional find
request is done because the custom settings is provided only by /namespaces/{Namespace}/services
GET request.
Request to add custom settings to the services
list API: epinio/epinio#2519
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.
@richard-cox we verified that the settings are correctly provided by services
list api, and are provided by CreateEditView
underneath component: epinio/epinio#2519 (comment)
Given that, I removed the extra request.
Signed-off-by: Francesco Torchia <[email protected]>
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.
just one comment, then we're good
Signed-off-by: Francesco Torchia <[email protected]>
Summary
Fixes #338
Fixes #296
Technical Notes
string
maps.Screenshots
View:
Create:
Edit: