yahoo-eng-team team mailing list archive
-
yahoo-eng-team team
-
Mailing list archive
-
Message #54821
[Bug 1517839] Re: Make CONF.set_override with parameter enforce_type=True by default
Reviewed: https://review.openstack.org/325749
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=e6a05382bd257e74a81a551f23dc86ccb9ee01c5
Submitter: Jenkins
Branch: master
commit e6a05382bd257e74a81a551f23dc86ccb9ee01c5
Author: ChangBo Guo(gcb) <eric.guo@xxxxxxxxxxxx>
Date: Mon Jun 6 14:47:12 2016 +0800
Set enforce_type=True in method flags
Current Nova uses method CONF.set_override to change config option's
value with designated value in unit test, but never check if the
designated vaule is valid. Each config option has a type like strOpt,
BoolOpt, etc. StrOpt with parameter choices only allows values in set of
choices. In short word, each config option has limitation for type and
value. In production code, oslo.conf can ensure user's input is valid,
but in unit test, test methods can pass if we use method
CONF.set_override without parameter enforce_type=True even we pass wrong
type or wrong value to config option.
This commit makes enforce_type=True in method flags in nova/test.py and
fix related violations.
Closes-Bug: #1517839
Change-Id: I105dabfec89b01d131257054fbb3669763af6ca9
** Changed in: nova
Status: In Progress => Fix Released
--
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1517839
Title:
Make CONF.set_override with parameter enforce_type=True by default
Status in Cinder:
In Progress
Status in cloudkitty:
Fix Released
Status in Designate:
Fix Released
Status in Glance:
New
Status in heat:
Fix Released
Status in OpenStack Identity (keystone):
Fix Released
Status in Manila:
Fix Released
Status in Murano:
Fix Released
Status in neutron:
Won't Fix
Status in OpenStack Compute (nova):
Fix Released
Status in oslo.config:
In Progress
Status in oslo.messaging:
Fix Released
Status in Rally:
Fix Released
Status in watcher:
Fix Released
Bug description:
1. Problems :
oslo_config provides method CONF.set_override[1] , developers usually use it to change config option's value in tests. That's convenient .
By default parameter enforce_type=False, it doesn't check any type or value of override. If set enforce_type=True , will check parameter
override's type and value. In production code(running time code), oslo_config always checks config option's value.
In short, we test and run code in different ways. so there's gap: config option with wrong type or invalid value can pass tests when
parameter enforce_type = False in consuming projects. that means some invalid or wrong tests are in our code base.
[1]
https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py#L2173
2. Proposal
1) Fix violations when enforce_type=True in each project.
2) Make method CONF.set_override with enforce_type=True by default
in oslo_config
You can find more details and comments in
https://etherpad.openstack.org/p/enforce_type_true_by_default
3. How to find violations in your projects.
1. Run tox -e py27
2. then modify oslo.config with enforce_type=True
cd .tox/py27/lib64/python2.7/site-packages/oslo_config
edit cfg.py with enforce_type=True
- def set_override(self, name, override, group=None, enforce_type=False):
+ def set_override(self, name, override, group=None, enforce_type=True):
3. Run tox -e py27 again, you will find violations.
To manage notifications about this bug go to:
https://bugs.launchpad.net/cinder/+bug/1517839/+subscriptions