openstack/db: add Trove configuration parameter APIs - #3813
Conversation
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Hi @stias! I brought some comments that I have left at the old PR. Will take another time to review it thoroughly.
| } | ||
|
|
||
| // ToParamCreateMap converts a CreateParamOpts struct into a request body. | ||
| func (opts CreateParamOpts) ToParamCreateMap() (map[string]any, error) { |
There was a problem hiding this comment.
You can use the existing CreateParamOpts struct here instead of creating a new one to build the request body. We avoid having an almost-duplicated struct. For example:
func (opts CreateParamOpts) ToParamCreateMap() (map[string]any, error) {
b, err := gophercloud.BuildRequestBody(opts, "configuration-parameter")
if err != nil {
return nil, err
}
if opts.RestartRequired != nil {
b["restart_required"] = restartRequiredToInt(opts.RestartRequired)
}
return b, nil
}Does it make sense?
| MaxSize *int `json:"max_size,omitempty"` | ||
| // Whether the database service needs to restart after this parameter | ||
| // changes. | ||
| RestartRequired bool `json:"restart_required"` |
There was a problem hiding this comment.
I would say that we need this field to be a *bool because we need to differentiate between the zero-value (false) and a not set value.
If a user does not set the RestartRequired, the marshaler will infer this field as false and pass it to Trove API. Since the field is not required, I believe it would be better to keep it as nil and let the Trove set the default value. Wdyt?
|
|
||
| // UpdateParamOpts represents options for updating a configuration parameter | ||
| // for a datastore version. | ||
| type UpdateParamOpts struct { |
There was a problem hiding this comment.
Ditto for ToParamCreateMap
github-actions
Bot
added
semver:minor
|
coverage: 63.601% (+0.06%) from 63.545% — stias:feature/trove-db-configuration-parameters into gophercloud:main |
| // Extract will retrieve created params from an operation result. | ||
| func (r CreateParamResult) Extract() ([]Param, error) { | ||
| var s struct { | ||
| Params []Param `json:"configuration-parameters"` |
There was a problem hiding this comment.
I know the docs at https://docs.openstack.org/api-ref/database/#id23 show the result wrapped in {"configuration-parameters": [...]} but it appears the doc is wrong if you look at the source code.
and
Having acceptance test would prove this works as expected (or not).
4 participants
For #3809
Split out from #3767.
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://github.com/openstack/trove/blob/stable/2026.1/trove/extensions/routes/mgmt.py#L73-L77
https://github.com/openstack/trove/blob/stable/2026.1/trove/extensions/mgmt/configuration/service.py#L32-L135
https://github.com/openstack/trove/blob/stable/2026.1/trove/extensions/mgmt/configuration/views.py#L21-L53