Thanks for the discussion! Addressing your points:
> When I see a new config file option being added already in a deprecated
> state, I have to wonder if it's the right fix: [...]
The usage of a deprecated config option is arbitrary (it may be not used),
but the semantics are actually correct [1] for the scenario of this issue
(as the config option is not available in future releases):
class oslo_config.cfg.Opt(...)
deprecated_for_removal – indicates whether this opt is planned for removal in a future release
deprecated_reason – indicates why this opt is planned for removal in a future release. [...]
deprecated_since – indicates which release this opt was deprecated in. [...]
> If we need a workaround, maybe it shouldn't be in the config file like
> that. It's opt-in, the default value is such that it doesn't change
> behavior, sounds like the config file is just a means for documentation.
I can appreciate your opinion (and have a similar take on this, in general),
however, the package/project goes with all options being in the config file
(with docs), including a dedicated 'workarounds' section, with many opt-ins:
(There are technical reasons this workaround is in the defaults section,
instead of workarounds: for integration with the juju charm, see MR [2].)
> If this is indeed the only way to address this, could it not be in the
> default config file, and perhaps just be a "hidden" option that will be
> honored in this version? Its documentation could be elsewhere.
Although there are technical means to achieve that suggestion (eg, d/rules
sed, or non-standard usage of the config option library in the source code):
- that does not seem to be what the users of this package expect; and
- hiding it could hinder someone affected to find out the solution needed.
So, I'd suggest that we continue with that the package/project already use.
...
Hopefully the points above address your questions and make the upload be
considered for SRU acceptance again, as-is.
Summary:
- there have been other config file changes in the past (~25% of uploads
after focal-release, as in #21);
- the package/project already describes all config options in nova.conf
(with documentation), including workarounds and deprecations, which are
correct/in-line with the release-scope of the introduced config option.
Hi Andreas o/
Thanks for the discussion! Addressing your points:
> When I see a new config file option being added already in a deprecated
> state, I have to wonder if it's the right fix: [...]
The usage of a deprecated config option is arbitrary (it may be not used),
but the semantics are actually correct [1] for the scenario of this issue
(as the config option is not available in future releases):
class oslo_config. cfg.Opt( ...)
deprecated_ for_removal – indicates whether this opt is planned for removal in a future release reason – indicates why this opt is planned for removal in a future release. [...] since – indicates which release this opt was deprecated in. [...]
deprecated_
deprecated_
> If we need a workaround, maybe it shouldn't be in the config file like
> that. It's opt-in, the default value is such that it doesn't change
> behavior, sounds like the config file is just a means for documentation.
I can appreciate your opinion (and have a similar take on this, in general),
however, the package/project goes with all options being in the config file
(with docs), including a dedicated 'workarounds' section, with many opt-ins:
$ grep -e '^\[' -e '^#[^ ]' nova.conf libvirt_ livesnapshot = false virt_lifecycle_ events = true group_policy_ check_upcall = false numa_live_ migration = false libvirt_ rbd_instance_ dir_cleanup = false fallback_ pcpu_query = false download_ image_if_ on_rbd = false native_ luksv1 = false volume_ local_attach = false disk_resource_ for_image_ cache = false
...
[workarounds]
#disable_rootwrap = false
#disable_
#handle_
#disable_
#enable_
#ensure_
#disable_
#never_
#disable_
#rbd_
#reserve_
...
(There are technical reasons this workaround is in the defaults section,
instead of workarounds: for integration with the juju charm, see MR [2].)
> If this is indeed the only way to address this, could it not be in the
> default config file, and perhaps just be a "hidden" option that will be
> honored in this version? Its documentation could be elsewhere.
Although there are technical means to achieve that suggestion (eg, d/rules
sed, or non-standard usage of the config option library in the source code):
- that does not seem to be what the users of this package expect; and
- hiding it could hinder someone affected to find out the solution needed.
So, I'd suggest that we continue with that the package/project already use.
...
Hopefully the points above address your questions and make the upload be
considered for SRU acceptance again, as-is.
Summary:
- there have been other config file changes in the past (~25% of uploads
after focal-release, as in #21);
- the package/project already describes all config options in nova.conf
(with documentation), including workarounds and deprecations, which are
correct/in-line with the release-scope of the introduced config option.
Thanks again,
Mauricio
[1] https:/ /docs.openstack .org/oslo. config/ ussuri/ reference/ api/oslo_ config. html#oslo_ config. cfg.ConfigOpts /code.launchpad .net/~mfo/ ubuntu/ +source/ nova/+git/ nova/+merge/ 447799/ comments/ 1198731
[2] https:/