From 64e217f885be1e60699efb3422a4b549ebbb800d Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Wed, 4 Jun 2014 10:35:13 -0700 Subject: [PATCH] Make JJB python 3 compatible Convert to use idioms that work for both python 3 and python 2.6+ and ensure that a suitable version of dependencies is included for python 3 compatibility. Update python-jenkins to 0.3.4 as the earliest version that supports python 3 without any known regressions. Add an extra parser check for missing 'command' due to changes in how argparse works under python 3. To access the first element of a dict in both python 2 and 3, 'next(iter(dict.items()))' is used as the standard idiom to replace 'dict.items()[0]' as 'items()' returns an iterator in python 3 which cannot be indexed. Using 'next(iter(..))' allows for both lists and iterators to be passed in without unnecessary conversion of iterators to lists which would be true of 'list(dict.items())[0]'. Original change which was reverted due to breaking use of job-groups is If4b35e2ceee8239379700e22eb79a3eaa04d6f0f. This replaces the previous conversion of 'dict.items()[0]' to 'dict.popitem()', which would result in removing a job-group when first called, thus defeating the benefit of being able to reference the group mulitple times. This usage has been replaced with 'next(iter(dict.items()))' as a non-modifying alternative that still avoids creating unnecessary copies of data while working for all supported versions of python. Change-Id: I37e3b67c043dadddb54e16ee584bde3f79e6a770 --- jenkins_jobs/builder.py | 22 +++-- jenkins_jobs/cmd.py | 13 +-- jenkins_jobs/local_yaml.py | 2 +- jenkins_jobs/modules/builders.py | 2 +- jenkins_jobs/modules/hipchat_notif.py | 6 +- jenkins_jobs/modules/publishers.py | 14 +-- jenkins_jobs/modules/scm.py | 10 +-- jenkins_jobs/modules/triggers.py | 4 +- jenkins_jobs/modules/zuul.py | 6 +- requirements.txt | 3 +- tests/base.py | 13 ++- tests/cmd/test_cmd.py | 37 ++++---- .../fixtures/jobgroups_multi_use.xml | 88 +++++++++++++++++++ .../fixtures/jobgroups_multi_use.yaml | 34 +++++++ 14 files changed, 192 insertions(+), 62 deletions(-) create mode 100644 tests/yamlparser/fixtures/jobgroups_multi_use.xml create mode 100644 tests/yamlparser/fixtures/jobgroups_multi_use.yaml diff --git a/jenkins_jobs/builder.py b/jenkins_jobs/builder.py index fc0294e32..2f20da952 100644 --- a/jenkins_jobs/builder.py +++ b/jenkins_jobs/builder.py @@ -17,6 +17,7 @@ import errno import os +import operator import sys import hashlib import yaml @@ -31,8 +32,9 @@ import logging import copy import itertools import fnmatch +import six from jenkins_jobs.errors import JenkinsJobsException -import local_yaml +import jenkins_jobs.local_yaml as local_yaml logger = logging.getLogger(__name__) MAGIC_MANAGE_STRING = "" @@ -82,7 +84,7 @@ def deep_format(obj, paramdict): # limitations on the values in paramdict - the post-format result must # still be valid YAML (so substituting-in a string containing quotes, for # example, is problematic). - if isinstance(obj, basestring): + if hasattr(obj, 'format'): try: result = re.match('^{obj:(?P\w+)}$', obj) if result is not None: @@ -142,7 +144,7 @@ class YamlParser(object): " not a {cls}".format(fname=getattr(fp, 'name', fp), cls=type(data))) for item in data: - cls, dfn = item.items()[0] + cls, dfn = next(iter(item.items())) group = self.data.get(cls, {}) if len(item.items()) > 1: n = None @@ -209,7 +211,7 @@ class YamlParser(object): for jobspec in project.get('jobs', []): if isinstance(jobspec, dict): # Singleton dict containing dict of job-specific params - jobname, jobparams = jobspec.items()[0] + jobname, jobparams = next(iter(jobspec.items())) if not isinstance(jobparams, dict): jobparams = {} else: @@ -225,7 +227,7 @@ class YamlParser(object): for group_jobspec in group['jobs']: if isinstance(group_jobspec, dict): group_jobname, group_jobparams = \ - group_jobspec.items()[0] + next(iter(group_jobspec.items())) if not isinstance(group_jobparams, dict): group_jobparams = {} else: @@ -275,7 +277,7 @@ class YamlParser(object): expanded_values = {} for (k, v) in values: if isinstance(v, dict): - inner_key = v.iterkeys().next() + inner_key = next(iter(v)) expanded_values[k] = inner_key expanded_values.update(v[inner_key]) else: @@ -295,6 +297,8 @@ class YamlParser(object): # us guarantee a group of parameters will not be added a # second time. uniq = json.dumps(expanded, sort_keys=True) + if six.PY3: + uniq = uniq.encode('utf-8') checksum = hashlib.md5(uniq).hexdigest() # Lookup the checksum @@ -364,7 +368,7 @@ class ModuleRegistry(object): Mod = entrypoint.load() mod = Mod(self) self.modules.append(mod) - self.modules.sort(lambda a, b: cmp(a.sequence, b.sequence)) + self.modules.sort(key=operator.attrgetter('sequence')) if mod.component_type is not None: self.modules_by_component_type[mod.component_type] = mod @@ -408,7 +412,7 @@ class ModuleRegistry(object): if isinstance(component, dict): # The component is a singleton dictionary of name: dict(args) - name, component_data = component.items()[0] + name, component_data = next(iter(component.items())) if template_data: # Template data contains values that should be interpolated # into the component definition @@ -629,7 +633,7 @@ class Builder(object): self.load_files(input_fn) self.parser.generateXML(names) - self.parser.jobs.sort(lambda a, b: cmp(a.name, b.name)) + self.parser.jobs.sort(key=operator.attrgetter('name')) for job in self.parser.jobs: if names and not matches(job.name, names): diff --git a/jenkins_jobs/cmd.py b/jenkins_jobs/cmd.py index 8aa8c94a7..929e2a29f 100755 --- a/jenkins_jobs/cmd.py +++ b/jenkins_jobs/cmd.py @@ -14,12 +14,11 @@ # under the License. import argparse -import ConfigParser +from six.moves import configparser, StringIO import logging import os import platform import sys -import cStringIO from jenkins_jobs.builder import Builder from jenkins_jobs.errors import JenkinsJobsException @@ -110,6 +109,8 @@ def main(argv=None): parser = create_parser() options = parser.parse_args(argv) + if not options.command: + parser.error("Must specify a 'command' to be performed") if (options.log_level is not None): options.log_level = getattr(logging, options.log_level.upper(), logger.getEffectiveLevel()) @@ -130,9 +131,9 @@ def setup_config_settings(options): 'jenkins_jobs.ini') if os.path.isfile(localconf): conf = localconf - config = ConfigParser.ConfigParser() + config = configparser.ConfigParser() ## Load default config always - config.readfp(cStringIO.StringIO(DEFAULT_CONF)) + config.readfp(StringIO(DEFAULT_CONF)) if os.path.isfile(conf): logger.debug("Reading config from {0}".format(conf)) conffp = open(conf, 'r') @@ -167,11 +168,11 @@ def execute(options, config): # https://bugs.launchpad.net/openstack-ci/+bug/1259631 try: user = config.get('jenkins', 'user') - except (TypeError, ConfigParser.NoOptionError): + except (TypeError, configparser.NoOptionError): user = None try: password = config.get('jenkins', 'password') - except (TypeError, ConfigParser.NoOptionError): + except (TypeError, configparser.NoOptionError): password = None builder = Builder(config.get('jenkins', 'url'), diff --git a/jenkins_jobs/local_yaml.py b/jenkins_jobs/local_yaml.py index a833bae07..689ae06ec 100644 --- a/jenkins_jobs/local_yaml.py +++ b/jenkins_jobs/local_yaml.py @@ -187,7 +187,7 @@ class LocalLoader(OrderedConstructor, yaml.Loader): self.add_constructor(yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, type(self).construct_yaml_map) - if isinstance(self.stream, file): + if hasattr(self.stream, 'name'): self.search_path.add(os.path.normpath( os.path.dirname(self.stream.name))) self.search_path.add(os.path.normpath(os.path.curdir)) diff --git a/jenkins_jobs/modules/builders.py b/jenkins_jobs/modules/builders.py index e72de822b..b9ca5e811 100644 --- a/jenkins_jobs/modules/builders.py +++ b/jenkins_jobs/modules/builders.py @@ -251,7 +251,7 @@ def ant(parser, xml_parent, data): if type(data) is str: # Support for short form: -ant: "target" data = {'targets': data} - for setting, value in sorted(data.iteritems()): + for setting, value in sorted(data.items()): if setting == 'targets': targets = XML.SubElement(ant, 'targets') targets.text = value diff --git a/jenkins_jobs/modules/hipchat_notif.py b/jenkins_jobs/modules/hipchat_notif.py index 988ca4c87..5ae825060 100644 --- a/jenkins_jobs/modules/hipchat_notif.py +++ b/jenkins_jobs/modules/hipchat_notif.py @@ -46,7 +46,7 @@ import xml.etree.ElementTree as XML import jenkins_jobs.modules.base import jenkins_jobs.errors import logging -import ConfigParser +from six.moves import configparser import sys logger = logging.getLogger(__name__) @@ -73,8 +73,8 @@ class HipChat(jenkins_jobs.modules.base.Base): if self.authToken == '': raise jenkins_jobs.errors.JenkinsJobsException( "Hipchat authtoken must not be a blank string") - except (ConfigParser.NoSectionError, - jenkins_jobs.errors.JenkinsJobsException), e: + except (configparser.NoSectionError, + jenkins_jobs.errors.JenkinsJobsException) as e: logger.fatal("The configuration file needs a hipchat section" + " containing authtoken:\n{0}".format(e)) sys.exit(1) diff --git a/jenkins_jobs/modules/publishers.py b/jenkins_jobs/modules/publishers.py index cd869c714..3ef4b2b95 100644 --- a/jenkins_jobs/modules/publishers.py +++ b/jenkins_jobs/modules/publishers.py @@ -439,7 +439,7 @@ def cloverphp(parser, xml_parent, data): metrics = data.get('metric-targets', []) # list of dicts to dict - metrics = dict(kv for m in metrics for kv in m.iteritems()) + metrics = dict(kv for m in metrics for kv in m.items()) # Populate defaults whenever nothing has been filled by user. for default in default_metrics.keys(): @@ -887,7 +887,7 @@ def xunit(parser, xml_parent, data): supported_types = [] for configured_type in data['types']: - type_name = configured_type.keys()[0] + type_name = next(iter(configured_type.keys())) if type_name not in implemented_types: logger.warn("Requested xUnit type '%s' is not yet supported", type_name) @@ -898,7 +898,7 @@ def xunit(parser, xml_parent, data): # Generate XML for each of the supported framework types xmltypes = XML.SubElement(xunit, 'types') for supported_type in supported_types: - framework_name = supported_type.keys()[0] + framework_name = next(iter(supported_type.keys())) xmlframework = XML.SubElement(xmltypes, types_to_plugin_types[framework_name]) @@ -922,9 +922,10 @@ def xunit(parser, xml_parent, data): "Unrecognized threshold, should be 'failed' or 'skipped'") continue elname = "org.jenkinsci.plugins.xunit.threshold.%sThreshold" \ - % t.keys()[0].title() + % next(iter(t.keys())).title() el = XML.SubElement(xmlthresholds, elname) - for threshold_name, threshold_value in t.values()[0].items(): + for threshold_name, threshold_value in \ + next(iter(t.values())).items(): # Normalize and craft the element name for this threshold elname = "%sThreshold" % threshold_name.lower().replace( 'new', 'New') @@ -3502,7 +3503,8 @@ def ruby_metrics(parser, xml_parent, data): XML.SubElement(el, 'metric').text = 'TOTAL_COVERAGE' else: XML.SubElement(el, 'metric').text = 'CODE_COVERAGE' - for threshold_name, threshold_value in t.values()[0].items(): + for threshold_name, threshold_value in \ + next(iter(t.values())).items(): elname = threshold_name.lower() XML.SubElement(el, elname).text = str(threshold_value) else: diff --git a/jenkins_jobs/modules/scm.py b/jenkins_jobs/modules/scm.py index 89e599b10..81a78ca9f 100644 --- a/jenkins_jobs/modules/scm.py +++ b/jenkins_jobs/modules/scm.py @@ -161,9 +161,9 @@ remoteName/\*') data['remotes'] = [{data.get('name', 'origin'): data.copy()}] for remoteData in data['remotes']: huser = XML.SubElement(user, 'hudson.plugins.git.UserRemoteConfig') - remoteName = remoteData.keys()[0] + remoteName = next(iter(remoteData.keys())) XML.SubElement(huser, 'name').text = remoteName - remoteParams = remoteData.values()[0] + remoteParams = next(iter(remoteData.values())) if 'refspec' in remoteParams: refspec = remoteParams['refspec'] else: @@ -368,7 +368,7 @@ def store(parser, xml_parent, data): pundles = XML.SubElement(scm, 'pundles') for pundle_spec in pundle_specs: pundle = XML.SubElement(pundles, '{0}.PundleSpec'.format(namespace)) - pundle_type = pundle_spec.keys()[0] + pundle_type = next(iter(pundle_spec)) pundle_name = pundle_spec[pundle_type] if pundle_type not in valid_pundle_types: raise JenkinsJobsException( @@ -507,9 +507,9 @@ def tfs(parser, xml_parent, data): server. :arg str login: The user name that is registered on the server. The user name must contain the name and the domain name. Entered as - domain\\\user or user\@domain (optional). + domain\\\\user or user\@domain (optional). **NOTE**: You must enter in at least two slashes for the - domain\\\user format in JJB YAML. It will be rendered normally. + domain\\\\user format in JJB YAML. It will be rendered normally. :arg str use-update: If true, Hudson will not delete the workspace at end of each build. This causes the artifacts from the previous build to remain when a new build starts. (default true) diff --git a/jenkins_jobs/modules/triggers.py b/jenkins_jobs/modules/triggers.py index a7c846fba..65f74607e 100644 --- a/jenkins_jobs/modules/triggers.py +++ b/jenkins_jobs/modules/triggers.py @@ -91,7 +91,7 @@ def build_gerrit_triggers(xml_parent, data): 'hudsontrigger.events' trigger_on_events = XML.SubElement(xml_parent, 'triggerOnEvents') - for config_key, tag_name in available_simple_triggers.iteritems(): + for config_key, tag_name in available_simple_triggers.items(): if data.get(config_key, False): XML.SubElement(trigger_on_events, '%s.%s' % (tag_namespace, tag_name)) @@ -453,7 +453,7 @@ def pollurl(parser, xml_parent, data): str(bool(check_content)).lower() content_types = XML.SubElement(entry, 'contentTypes') for entry in check_content: - type_name = entry.keys()[0] + type_name = next(iter(entry.keys())) if type_name not in valid_content_types: raise JenkinsJobsException('check-content must be one of : %s' % ', '.join(valid_content_types. diff --git a/jenkins_jobs/modules/zuul.py b/jenkins_jobs/modules/zuul.py index cd823039a..79b428f63 100644 --- a/jenkins_jobs/modules/zuul.py +++ b/jenkins_jobs/modules/zuul.py @@ -35,6 +35,8 @@ The above URL is the default. http://ci.openstack.org/zuul/launchers.html#zuul-parameters """ +import itertools + def zuul(): """yaml: zuul @@ -152,8 +154,8 @@ class Zuul(jenkins_jobs.modules.base.Base): def handle_data(self, parser): changed = False - jobs = (parser.data.get('job', {}).values() + - parser.data.get('job-template', {}).values()) + jobs = itertools.chain(parser.data.get('job', {}).values(), + parser.data.get('job-template', {}).values()) for job in jobs: triggers = job.get('triggers') if not triggers: diff --git a/requirements.txt b/requirements.txt index 535be29e8..7e1e832f6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ argparse ordereddict +six>=1.5.2 PyYAML -python-jenkins +python-jenkins>=0.3.4 pbr>=0.8.2,<1.0 diff --git a/tests/base.py b/tests/base.py index c684e8ea0..8eeb8aefe 100644 --- a/tests/base.py +++ b/tests/base.py @@ -26,7 +26,7 @@ import json import operator import testtools import xml.etree.ElementTree as XML -from ConfigParser import ConfigParser +from six.moves import configparser import jenkins_jobs.local_yaml as yaml from jenkins_jobs.builder import XmlJob, YamlParser, ModuleRegistry from jenkins_jobs.modules import (project_flow, @@ -86,7 +86,7 @@ class BaseTestCase(object): def _read_yaml_content(self): yaml_filepath = os.path.join(self.fixtures_path, self.in_filename) - with file(yaml_filepath, 'r') as yaml_file: + with open(yaml_filepath, 'r') as yaml_file: yaml_content = yaml.load(yaml_file) return yaml_content @@ -118,8 +118,7 @@ class BaseTestCase(object): pub.gen_xml(parser, xml_project, yaml_content) # Prettify generated XML - pretty_xml = unicode(XmlJob(xml_project, 'fixturejob').output(), - 'utf-8') + pretty_xml = XmlJob(xml_project, 'fixturejob').output().decode('utf-8') self.assertThat( pretty_xml, @@ -137,7 +136,7 @@ class SingleJobTestCase(BaseTestCase): yaml_filepath = os.path.join(self.fixtures_path, self.in_filename) if self.conf_filename: - config = ConfigParser() + config = configparser.ConfigParser() conf_filepath = os.path.join(self.fixtures_path, self.conf_filename) config.readfp(open(conf_filepath)) @@ -152,8 +151,8 @@ class SingleJobTestCase(BaseTestCase): parser.jobs.sort(key=operator.attrgetter('name')) # Prettify generated XML - pretty_xml = unicode("\n".join(job.output() for job in parser.jobs), - 'utf-8') + pretty_xml = u"\n".join(job.output().decode('utf-8') + for job in parser.jobs) self.assertThat( pretty_xml, diff --git a/tests/cmd/test_cmd.py b/tests/cmd/test_cmd.py index d7e0ebaeb..94cb2093e 100644 --- a/tests/cmd/test_cmd.py +++ b/tests/cmd/test_cmd.py @@ -1,6 +1,6 @@ import os -import ConfigParser -import cStringIO +from six.moves import configparser, StringIO +import io import codecs import mock import testtools @@ -22,15 +22,15 @@ class CmdTests(testtools.TestCase): User passes no args, should fail with SystemExit """ with mock.patch('sys.stderr'): - self.assertRaises(SystemExit, self.parser.parse_args, []) + self.assertRaises(SystemExit, cmd.main, []) def test_non_existing_config_dir(self): """ Run test mode and pass a non-existing configuration directory """ args = self.parser.parse_args(['test', 'foo']) - config = ConfigParser.ConfigParser() - config.readfp(cStringIO.StringIO(cmd.DEFAULT_CONF)) + config = configparser.ConfigParser() + config.readfp(StringIO(cmd.DEFAULT_CONF)) self.assertRaises(IOError, cmd.execute, args, config) def test_non_existing_config_file(self): @@ -38,8 +38,8 @@ class CmdTests(testtools.TestCase): Run test mode and pass a non-existing configuration file """ args = self.parser.parse_args(['test', 'non-existing.yaml']) - config = ConfigParser.ConfigParser() - config.readfp(cStringIO.StringIO(cmd.DEFAULT_CONF)) + config = configparser.ConfigParser() + config.readfp(StringIO(cmd.DEFAULT_CONF)) self.assertRaises(IOError, cmd.execute, args, config) def test_non_existing_job(self): @@ -52,8 +52,8 @@ class CmdTests(testtools.TestCase): 'cmd-001.yaml'), 'invalid']) args.output_dir = mock.MagicMock() - config = ConfigParser.ConfigParser() - config.readfp(cStringIO.StringIO(cmd.DEFAULT_CONF)) + config = configparser.ConfigParser() + config.readfp(StringIO(cmd.DEFAULT_CONF)) cmd.execute(args, config) # probably better to fail here def test_valid_job(self): @@ -65,8 +65,8 @@ class CmdTests(testtools.TestCase): 'cmd-001.yaml'), 'foo-job']) args.output_dir = mock.MagicMock() - config = ConfigParser.ConfigParser() - config.readfp(cStringIO.StringIO(cmd.DEFAULT_CONF)) + config = configparser.ConfigParser() + config.readfp(StringIO(cmd.DEFAULT_CONF)) cmd.execute(args, config) # probably better to fail here def test_console_output(self): @@ -74,15 +74,14 @@ class CmdTests(testtools.TestCase): Run test mode and verify that resulting XML gets sent to the console. """ - console_out = cStringIO.StringIO() + console_out = io.BytesIO() with mock.patch('sys.stdout', console_out): cmd.main(['test', os.path.join(self.fixtures_path, 'cmd-001.yaml')]) - xml_content = u"%s" % codecs.open(os.path.join(self.fixtures_path, - 'cmd-001.xml'), - 'r', - 'utf-8').read() - self.assertEqual(console_out.getvalue(), xml_content) + xml_content = codecs.open(os.path.join(self.fixtures_path, + 'cmd-001.xml'), + 'r', 'utf-8').read() + self.assertEqual(console_out.getvalue().decode('utf-8'), xml_content) def test_config_with_test(self): """ @@ -121,8 +120,8 @@ class CmdTests(testtools.TestCase): args = self.parser.parse_args(['test', '-r', '/jjb_configs']) args.output_dir = mock.MagicMock() - config = ConfigParser.ConfigParser() - config.readfp(cStringIO.StringIO(cmd.DEFAULT_CONF)) + config = configparser.ConfigParser() + config.readfp(StringIO(cmd.DEFAULT_CONF)) cmd.execute(args, config) # probably better to fail here update_job_mock.assert_called_with(paths, [], output=args.output_dir) diff --git a/tests/yamlparser/fixtures/jobgroups_multi_use.xml b/tests/yamlparser/fixtures/jobgroups_multi_use.xml new file mode 100644 index 000000000..a1da87a90 --- /dev/null +++ b/tests/yamlparser/fixtures/jobgroups_multi_use.xml @@ -0,0 +1,88 @@ + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + #!/usr/bin/env python +# +print("Doing something cool with python") + + + + + + + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + #!/usr/bin/env python +# +print("Doing something cool with python") + + + + + + + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + #!/usr/bin/env python +# +print("Doing something else cool with python") + + + + + + + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + #!/usr/bin/env python +# +print("Doing something else cool with python") + + + + + + diff --git a/tests/yamlparser/fixtures/jobgroups_multi_use.yaml b/tests/yamlparser/fixtures/jobgroups_multi_use.yaml new file mode 100644 index 000000000..e34b5e933 --- /dev/null +++ b/tests/yamlparser/fixtures/jobgroups_multi_use.yaml @@ -0,0 +1,34 @@ +- job-group: + name: multiple_jobs + jobs: + - 'job_one_{version}': + - 'job_two_{version}': + +- project: + name: multiple_1.2 + version: 1.2 + jobs: + - multiple_jobs + +- project: + name: multiple_1.3 + version: 1.3 + jobs: + - multiple_jobs + +- job-template: + name: 'job_one_{version}' + builders: + - shell: | + #!/usr/bin/env python + # + print("Doing something cool with python") + +- job-template: + name: 'job_two_{version}' + builders: + - shell: | + #!/usr/bin/env python + # + print("Doing something else cool with python") +