jjb/jenkins-job-builder
Code Issues Proposed changes

No warn on old plugin conf format not in use

Browse Source 
Use a simple object to ensure matching against any default value is
possible to distinguish been not being set by a user versus use of
a negative default value passed.

This prevents triggering an incorrect notification to the user when
the code is querying for plugin options that could override the
default behaviour and the compatibility code attempts to look up an non
existing section. It would fail to distinguish between no section
present and current value to return matching the default value
suggested.

Change-Id: I5597c2628ccb5a4282a97a4ce5d3bbe41bd9eebb
This commit is contained in:
Darragh Bailey 2017-12-15 14:18:11 +00:00
parent c4aab5c22d
commit 630fc6e88a
3 changed files with 41 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
jenkins_jobs/config.py
View File

@ -51,6 +51,7 @@ query_plugins_info=True
CONFIG_REQUIRED_MESSAGE = ("A valid configuration file is required. "
"No configuration file passed.")
_NOTSET = object()
class JJBConfig(object):
@ -354,10 +355,13 @@ class JJBConfig(object):
# plugin configuration format in their config. This code should be
# removed in future versions of JJB after 2.0.
if value is default:
value = self.get_module_config(plugin, key, default)
logger.warning(
"Defining plugin configuration using [" + plugin + "] is"
" deprecated. The recommended way to define plugins now is by"
" configuring [plugin \"" + plugin + "\"]")
old_value = self.get_module_config(plugin, key, _NOTSET)
# only log warning if detected a plugin config setting.
if old_value is not _NOTSET:
value = old_value
logger.warning(
"Defining plugin configuration using [" + plugin + "] is "
"deprecated. The recommended way to define plugins now is "
"by configuring [plugin \"" + plugin + "\"]")
return value

5
tests/cmd/fixtures/plugin_warning.ini Normal file
View File

@ -0,0 +1,5 @@
[old_plugin]
setting = some value
[plugin "new_plugin"]
setting = some value

27
tests/cmd/test_config.py
View File

@ -77,6 +77,33 @@ class TestConfigs(CmdTestsBase):
jenkins_jobs = entry.JenkinsJobs(args)
self.assertRaises(IOError, jenkins_jobs.execute)
def test_config_old_plugin_format_warning(self):
"""
Run test mode and check that old plugin settings result
in a warning, while ensuring that missing sections do not
trigger the same warning if a default value is provided.
"""
args = ['--conf',
os.path.join(self.fixtures_path, 'plugin_warning.ini'),
'test', 'foo']
jenkins_jobs = entry.JenkinsJobs(args)
jenkins_jobs.jjb_config.get_plugin_config(
'old_plugin', 'setting', True)
jenkins_jobs.jjb_config.get_plugin_config(
'old_plugin_no_conf', 'setting', True)
jenkins_jobs.jjb_config.get_plugin_config(
'new_plugin', 'setting')
self.assertIn(
'Defining plugin configuration using [old_plugin] is deprecated',
self.logger.output)
self.assertNotIn(
'Defining plugin configuration using [old_plugin_no_conf] is '
'deprecated',
self.logger.output)
self.assertNotIn(
'Defining plugin configuration using [new_plugin] is deprecated',
self.logger.output)
def test_config_options_not_replaced_by_cli_defaults(self):
"""
Run test mode and check config settings from conf file retained