Support ExtraTagInfo, ExtraFieldInfo, and data verification #48
Conversation
PhanLe1010
force-pushed
the
47
branch
9 times, most recently
from
23e2f90
to
8965311
Compare
There was a problem hiding this comment.
In general, LGTM.
Let's merge this until @c3y1huang finishes the review.
| ReleaseDate string `json:"releaseDate"` | ||
| MinUpgradableVersion string `json:"minUpgradableVersion"` // can be empty or semantic versioning | ||
| Tags []string `json:"tags"` | ||
| ExtraInfo map[string]string `json:"extraInfo"` |
There was a problem hiding this comment.
The project maintainers can use this ExtraInfo to send back to the clients additional information such as minKubernetesVersion/maxKubernetesVersion.
Longhorn project doesn't use this feature though
upgraderesponder/service.go
Outdated
| } | ||
| case "float", "boolean": | ||
| default: | ||
| return fmt.Errorf("schema %v has invalid data type %v", schemaName, schema.DataType) |
There was a problem hiding this comment.
| return fmt.Errorf("schema %v has invalid data type %v", schemaName, schema.DataType) | |
| return fmt.Errorf("field schema %v has invalid data type %v", schemaName, schema.DataType) |
There was a problem hiding this comment.
Thank you. Fixed
upgraderesponder/service.go
Outdated
| MaxLen int `json:"maxLen"` | ||
| } | ||
|
|
||
| func (sc *Schema) Validate(value interface{}) bool { |
There was a problem hiding this comment.
Should we add some logs if the value doesn't pass the validation?
There was a problem hiding this comment.
Sounds helpful since the method just returns bool.
There was a problem hiding this comment.
I thought about it but worried that the logs might explode due to hundreds of requests per second. WDYT?
There was a problem hiding this comment.
Then let's add trace log instead.
There was a problem hiding this comment.
Added the log at debugging level. Thank you!
longhorn/longhorn#5235 https://github.com/longhorn/upgrade-responder/issues/47 Signed-off-by: Phan Le <phan.le@suse.com> Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>
|
@PhanLe1010 let's update the dashboard after 1.5.0 out. |
longhorn/longhorn#5235
https://github.com/longhorn/upgrade-responder/issues/47