Skip to content
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

script_tools.py --disable-locking option not recognized correctly #29

Open
kwaegema opened this issue Dec 14, 2015 · 2 comments
Open

script_tools.py --disable-locking option not recognized correctly #29

kwaegema opened this issue Dec 14, 2015 · 2 comments

Comments

@kwaegema
Copy link
Member

[root@ceph001 ~]# ./ceph-smart_health -d --disable-locking
2015-12-14 17:50:16,919 DEBUG ceph-smart_health.ExtOption MainThread changed loglevel to DEBUG, previous state: (ceph-smart_health.SimpleOptionParser, NOTSET), (ceph-smart_health.CEPH-SMART_HEALTH, NOTSET), (ceph-smart_health.ExtOptionGroup, NOTSET), (ceph-smart_health.vsc.utils.missing, NOTSET), (ceph-smart_health.main, NOTSET), (ceph-smart_health.vsc.utils.nagios, NOTSET), (ceph-smart_health.vsc.utils.lock, NOTSET), (ceph-smart_health.ExtOption, NOTSET), (ceph-smart_health, NOTSET)
2015-12-14 17:50:16,919 DEBUG ceph-smart_health.CEPH-SMART_HEALTH MainThread parseoptions: options from environment []
2015-12-14 17:50:16,919 DEBUG ceph-smart_health.CEPH-SMART_HEALTH MainThread parseoptions: options from commandline ['-d', '--disable-locking']
2015-12-14 17:50:16,919 DEBUG ceph-smart_health.CEPH-SMART_HEALTH MainThread Found options {'info': False, 'disable_locking': False, 'cache': '/tmp/smart_ceph_health.json.gz', 'help': None, 'dry_run': False, 'nagios_report': False, 'nagios_check_interval_threshold': 0, 'locking_filename': '/var/lock/ceph-smart_health.lock', 'quiet': False, 'configfiles': None, 'debug': True, 'ignoreconfigfiles': None, 'nagios_check_filename': '/var/cache/ceph-smart_health.nagios.json.gz', 'ha': None} args []

also not shown when running -H.
But it doesn't give an error that it is unrecognized or something (like --disable-bla would give), but it still is set to false. the dry-run option, declared in same place ,works as expected.
Running 1.7.6

options = {
    'cache': ('the location to store the cache with previous release hold data', None, 'store',
              CEPH_HEALTH_CACHE_FILE)
}
opts = ExtendedSimpleOption(options)

print opts.options.disable_locking #-> False
@JensTimmerman
Copy link
Contributor

-H shows you:
Boolean options support disable prefix to do the inverse of the action, e.g. option --someopt also supports --disable-someopt.

So agreed, the disable-locking option is just very confusing, and should not exist.
it should be renamed locking in vsc.utils.script_tools
so the --disable-locking options would still exist and work as expected.

@stdweird
Copy link
Member

yes, option names shouldn't have any negative action, the option should be named locking, type store_true and default True.
but the reported issue is a general option bug, it should not allow an option name with the ENABLE/DISABLE prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants