From d62fcc8ca4702c31e029713877ef2b0338a583a9 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 22 Sep 2017 16:24:59 +0100 Subject: [PATCH] Switch param order from yaml conf to plugin specific Migrate from a global config __future__ to control behaviour to plugin specific options to decide on whether to take the param order from yaml when using the trigger-parameterized-builds plugin or when plugins are making use of it as well. Adjust the config retrieval to support a default value to simplify logic around ensuring the behaviour matches 'true' by default. Remove some redundant logic in helper module handling a default value lookup and add some additional conf files to continue having some tests using the old behaviour. Remove old conf files setting the __futures__.param_order_from_yaml to true as this is now the default under the correct plugin setting. Change-Id: Ibd5f549b6d626bacaaa4221015a70aaf03626b00 --- jenkins_jobs/config.py | 23 ++++---- jenkins_jobs/modules/helpers.py | 56 ++++++++----------- jenkins_jobs/modules/publishers.py | 5 +- tests/publishers/fixtures/pipeline002.conf | 2 + tests/publishers/fixtures/pipeline003.conf | 2 + .../parameter-override-ordering.conf | 2 - .../trigger_parameterized_builds001.conf | 2 + .../trigger_parameterized_builds003.conf | 2 + .../parameter-override-ordering-001.conf | 2 - .../parameter-override-ordering-002.conf | 2 - 10 files changed, 46 insertions(+), 52 deletions(-) create mode 100644 tests/publishers/fixtures/pipeline002.conf create mode 100644 tests/publishers/fixtures/pipeline003.conf delete mode 100644 tests/publishers/fixtures/trigger_parameterized_builds/parameter-override-ordering.conf create mode 100644 tests/publishers/fixtures/trigger_parameterized_builds001.conf create mode 100644 tests/publishers/fixtures/trigger_parameterized_builds003.conf delete mode 100644 tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-001.conf delete mode 100644 tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-002.conf diff --git a/jenkins_jobs/config.py b/jenkins_jobs/config.py index 5fa1ccaa5..58016051f 100644 --- a/jenkins_jobs/config.py +++ b/jenkins_jobs/config.py @@ -325,33 +325,36 @@ class JJBConfig(object): 'allow_empty_variables', str(self.yamlparser['allow_empty_variables'])) - def get_module_config(self, section, key): + def get_module_config(self, section, key, default=None): """ Given a section name and a key value, return the value assigned to the key in the JJB .ini file if it exists, otherwise emit a warning indicating that the value is not set. Default value returned if no value is set in the file will be a blank string. """ - result = '' + result = default try: result = self.config_parser.get( section, key ) except (configparser.NoSectionError, configparser.NoOptionError, JenkinsJobsException) as e: - logger.warning("You didn't set a " + key + - " neither in the yaml job definition nor in" + - " the " + section + " section, blank default" + - " value will be applied:\n{0}".format(e)) + # use of default ignores missing sections/options + if result is None: + logger.warning( + "You didn't set a %s neither in the yaml job definition " + "nor in the %s section, blank default value will be " + "applied:\n%s", key, section, e) return result - def get_plugin_config(self, plugin, key): - value = self.get_module_config('plugin "{}"'.format(plugin), key) + def get_plugin_config(self, plugin, key, default=None): + value = self.get_module_config('plugin "{}"'.format(plugin), key, + default) # Backwards compatibility for users who have not switched to the new # plugin configuration format in their config. This code should be # removed in future versions of JJB after 2.0. - if not value: - value = self.get_module_config(plugin, key) + 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" diff --git a/jenkins_jobs/modules/helpers.py b/jenkins_jobs/modules/helpers.py index 6ff5f0f09..ee8e0f52c 100644 --- a/jenkins_jobs/modules/helpers.py +++ b/jenkins_jobs/modules/helpers.py @@ -14,7 +14,6 @@ import logging -import six import xml.etree.ElementTree as XML from jenkins_jobs.errors import InvalidAttributeError @@ -251,10 +250,7 @@ def findbugs_settings(xml_parent, data): def get_value_from_yaml_or_config_file(key, section, data, jjb_config): - result = data.get(key, '') - if result == '': - result = jjb_config.get_plugin_config(section, key) - return result + return jjb_config.get_plugin_config(section, key, data.get(key, '')) def cloudformation_region_dict(): @@ -469,39 +465,31 @@ def test_fairy_common(xml_element, data): convert_mapping_to_xml(xml_element, data, mappings, fail_required=True) -def trigger_get_parameter_order(registry): +def trigger_get_parameter_order(registry, plugin): logger = logging.getLogger("%s:trigger_get_parameter_order" % __name__) - # original order - param_order = [ - 'predefined-parameters', - 'git-revision', - 'property-file', - 'current-parameters', - 'node-parameters', - 'svn-revision', - 'restrict-matrix-project', - 'node-label-name', - 'node-label', - 'boolean-parameters', - ] - try: - if registry.jjb_config.config_parser.getboolean( - '__future__', 'param_order_from_yaml'): - param_order = None - except six.moves.configparser.NoSectionError: - pass - - if param_order: + if str(registry.jjb_config.get_plugin_config( + plugin, 'param_order_from_yaml', True)).lower() == 'false': logger.warning( - "Using deprecated order for parameter sets in " - "triggered-parameterized-builds. This will be changed in a future " - "release to inherit the order from the user defined yaml. To " - "enable this behaviour immediately, set the config option " - "'__future__.param_order_from_yaml' to 'true' and change the " - "input job configuration to use the desired order") + "Using deprecated order for parameter sets in %s. It is " + "recommended that you update your job definition instead of " + "enabling use of the old hardcoded order", plugin) - return param_order + # deprecated order + return [ + 'predefined-parameters', + 'git-revision', + 'property-file', + 'current-parameters', + 'node-parameters', + 'svn-revision', + 'restrict-matrix-project', + 'node-label-name', + 'node-label', + 'boolean-parameters', + ] + + return None def trigger_project(tconfigs, project_def, param_order=None): diff --git a/jenkins_jobs/modules/publishers.py b/jenkins_jobs/modules/publishers.py index e71341882..a14349f55 100644 --- a/jenkins_jobs/modules/publishers.py +++ b/jenkins_jobs/modules/publishers.py @@ -532,7 +532,8 @@ def trigger_parameterized_builds(registry, xml_parent, data): tbuilder = XML.SubElement(xml_parent, pt_prefix + 'BuildTrigger') configs = XML.SubElement(tbuilder, 'configs') - param_order = helpers.trigger_get_parameter_order(registry) + param_order = helpers.trigger_get_parameter_order( + registry, 'trigger-parameterized-builds') for project_def in data: tconfig = XML.SubElement(configs, pt_prefix + 'BuildTriggerConfig') @@ -1839,7 +1840,7 @@ def pipeline(registry, xml_parent, data): See 'samples/pipeline.yaml' for an example pipeline implementation. """ logger = logging.getLogger("%s:pipeline" % __name__) - param_order = helpers.trigger_get_parameter_order(registry) + param_order = helpers.trigger_get_parameter_order(registry, 'pipeline') if 'project' in data: logger.warning( diff --git a/tests/publishers/fixtures/pipeline002.conf b/tests/publishers/fixtures/pipeline002.conf new file mode 100644 index 000000000..7b1ff4932 --- /dev/null +++ b/tests/publishers/fixtures/pipeline002.conf @@ -0,0 +1,2 @@ +[plugin "pipeline"] +param_order_from_yaml = False diff --git a/tests/publishers/fixtures/pipeline003.conf b/tests/publishers/fixtures/pipeline003.conf new file mode 100644 index 000000000..7b1ff4932 --- /dev/null +++ b/tests/publishers/fixtures/pipeline003.conf @@ -0,0 +1,2 @@ +[plugin "pipeline"] +param_order_from_yaml = False diff --git a/tests/publishers/fixtures/trigger_parameterized_builds/parameter-override-ordering.conf b/tests/publishers/fixtures/trigger_parameterized_builds/parameter-override-ordering.conf deleted file mode 100644 index 8971c8054..000000000 --- a/tests/publishers/fixtures/trigger_parameterized_builds/parameter-override-ordering.conf +++ /dev/null @@ -1,2 +0,0 @@ -[__future__] -param_order_from_yaml = True diff --git a/tests/publishers/fixtures/trigger_parameterized_builds001.conf b/tests/publishers/fixtures/trigger_parameterized_builds001.conf new file mode 100644 index 000000000..384d62bff --- /dev/null +++ b/tests/publishers/fixtures/trigger_parameterized_builds001.conf @@ -0,0 +1,2 @@ +[plugin "trigger-parameterized-builds"] +param_order_from_yaml = False diff --git a/tests/publishers/fixtures/trigger_parameterized_builds003.conf b/tests/publishers/fixtures/trigger_parameterized_builds003.conf new file mode 100644 index 000000000..384d62bff --- /dev/null +++ b/tests/publishers/fixtures/trigger_parameterized_builds003.conf @@ -0,0 +1,2 @@ +[plugin "trigger-parameterized-builds"] +param_order_from_yaml = False diff --git a/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-001.conf b/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-001.conf deleted file mode 100644 index 8971c8054..000000000 --- a/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-001.conf +++ /dev/null @@ -1,2 +0,0 @@ -[__future__] -param_order_from_yaml = True diff --git a/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-002.conf b/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-002.conf deleted file mode 100644 index 8971c8054..000000000 --- a/tests/yamlparser/fixtures/trigger_parameterized_builds/parameter-override-ordering-002.conf +++ /dev/null @@ -1,2 +0,0 @@ -[__future__] -param_order_from_yaml = True