Untangle argparser and config objects

Ensure that argparser object only makes use of the JJBConfig object
instead of leaking a command line parser into a config object for API
usage.

Take care to use 'None' as default for store_true/store_false options
in order to ensure that the config file defaults are only overridden
when CLI options are explicitly set.

Change-Id: I4066ad750f9893759c2e9bdfde425fafacc7e672
This commit is contained in:
Darragh Bailey 2016-07-08 19:10:40 +01:00
parent 033d60d0c9
commit 70334fbf4b
7 changed files with 169 additions and 128 deletions

@ -13,15 +13,21 @@
# License for the specific language governing permissions and limitations
# under the License.
import io
import os
import logging
import platform
import sys
from jenkins_jobs import cmd
from jenkins_jobs import version
from jenkins_jobs.cli.parser import create_parser
from jenkins_jobs.config import JJBConfig
import yaml
logging.basicConfig(level=logging.INFO)
from jenkins_jobs.cli.parser import create_parser
from jenkins_jobs import cmd
from jenkins_jobs.config import JJBConfig
from jenkins_jobs import utils
from jenkins_jobs import version
logger = logging.getLogger()
def __version__():
@ -49,24 +55,70 @@ class JenkinsJobs(object):
def __init__(self, args=None, **kwargs):
if args is None:
args = []
parser = create_parser()
options = parser.parse_args(args)
self.parser = create_parser()
self.options = self.parser.parse_args(args)
self.jjb_config = JJBConfig(arguments=options, **kwargs)
self.jjb_config.do_magical_things()
self.jjb_config = JJBConfig(self.options.conf, **kwargs)
if not options.command:
parser.error("Must specify a 'command' to be performed")
if not self.options.command:
self.parser.error("Must specify a 'command' to be performed")
logger = logging.getLogger()
if (options.log_level is not None):
options.log_level = getattr(logging,
options.log_level.upper(),
logger.getEffectiveLevel())
logger.setLevel(options.log_level)
if (self.options.log_level is not None):
self.options.log_level = getattr(logging,
self.options.log_level.upper(),
logger.getEffectiveLevel())
logger.setLevel(self.options.log_level)
self._parse_additional()
self.jjb_config.validate()
def _parse_additional(self):
for opt in ['ignore_cache', 'user', 'password',
'allow_empty_variables']:
opt_val = getattr(self.options, opt, None)
if opt_val is not None:
setattr(self.jjb_config, opt, opt_val)
if getattr(self.options, 'plugins_info_path', None) is not None:
with io.open(self.options.plugins_info_path, 'r',
encoding='utf-8') as yaml_file:
plugins_info = yaml.load(yaml_file)
if not isinstance(plugins_info, list):
self.parser.error("{0} must contain a Yaml list!".format(
self.options.plugins_info_path))
self.jjb_config.plugins_info = plugins_info
if getattr(self.options, 'path', None):
if hasattr(self.options.path, 'read'):
logger.debug("Input file is stdin")
if self.options.path.isatty():
if platform.system() == 'Windows':
key = 'CTRL+Z'
else:
key = 'CTRL+D'
logger.warn("Reading configuration from STDIN. "
"Press %s to end input.", key)
else:
# take list of paths
self.options.path = self.options.path.split(os.pathsep)
do_recurse = (getattr(self.options, 'recursive', False) or
self.jjb_config.recursive)
excludes = ([e for elist in self.options.exclude
for e in elist.split(os.pathsep)] or
self.jjb_config.excludes)
paths = []
for path in self.options.path:
if do_recurse and os.path.isdir(path):
paths.extend(utils.recurse_path(path, excludes))
else:
paths.append(path)
self.options.path = paths
def execute(self):
cmd.execute(self.jjb_config)
cmd.execute(self.options, self.jjb_config)
def main():

@ -43,7 +43,7 @@ def create_parser():
'--ignore-cache',
action='store_true',
dest='ignore_cache',
default=False,
default=None,
help='''ignore the cache and update the jobs anyhow (that will only
flush the specified jobs cache)''')
parser.add_argument(

@ -53,9 +53,8 @@ def confirm(question):
sys.exit('Aborted')
def execute(jjb_config):
def execute(options, jjb_config):
config = jjb_config.config_parser
options = jjb_config.arguments
builder = Builder(config.get('jenkins', 'url'),
jjb_config.user,

@ -17,15 +17,11 @@
import io
import logging
import platform
import os
import yaml
from six.moves import configparser, StringIO
from jenkins_jobs import builder
from jenkins_jobs import utils
from jenkins_jobs.cli.parser import create_parser
from jenkins_jobs.errors import JenkinsJobsException
__all__ = [
@ -59,8 +55,7 @@ class JJBConfigException(JenkinsJobsException):
class JJBConfig(object):
def __init__(self, config_filename=None, arguments=None,
config_file_required=False):
def __init__(self, config_filename=None, config_file_required=False):
"""
The JJBConfig class is intended to encapsulate and resolve priority
@ -70,15 +65,12 @@ class JJBConfig(object):
It also allows users of JJB-as-an-API to create minimally valid
configuration and easily make minor modifications to default values
without strictly adhering to the confusing setup (see the
do_magical_things method, the behavior of which largely lived in the
cmd.execute method previously) necessary for the jenkins-jobs command
line tool.
without strictly adhering to the confusing setup (see the _setup
method, the behavior of which largely lived in the cmd.execute method
previously) necessary for the jenkins-jobs command line tool.
:arg str config_filename: Name of configuration file on which to base
this config object.
:arg dict arguments: A argparse.Namespace object as produced by the
jenkins-jobs argument parser.
:arg bool config_file_required: Allows users of the JJBConfig class to
decide whether or not it's really necessary for a config file to be
passed in when creating an instance. This has two effects on the
@ -92,10 +84,6 @@ class JJBConfig(object):
config_parser = self._init_defaults()
if arguments is None:
jenkins_jobs_parser = create_parser()
arguments = jenkins_jobs_parser.parse_args(['test'])
global_conf = '/etc/jenkins_jobs/jenkins_jobs.ini'
user_conf = os.path.join(os.path.expanduser('~'), '.config',
'jenkins_jobs', 'jenkins_jobs.ini')
@ -105,9 +93,6 @@ class JJBConfig(object):
if config_filename is not None:
conf = config_filename
elif hasattr(arguments, 'conf') and arguments.conf is not None:
conf = arguments.conf
elif config_file_required:
if os.path.isfile(local_conf):
conf = local_conf
@ -124,14 +109,22 @@ class JJBConfig(object):
if config_file_required:
raise e
else:
logger.warn("""Config file, {0}, not found. Using default
config values.""".format(conf))
logger.warn("Config file, {0}, not found. Using default "
"config values.".format(conf))
if config_fp is not None:
config_parser.readfp(config_fp)
self.config_parser = config_parser
self.arguments = arguments
self.ignore_cache = False
self.user = None
self.password = None
self.plugins_info = None
self.timeout = builder._DEFAULT_TIMEOUT
self.allow_empty_variables = None
self._setup()
def _init_defaults(self):
""" Initialize default configuration values using DEFAULT_CONF
@ -155,18 +148,14 @@ class JJBConfig(object):
return config_fp
def do_magical_things(self):
def _setup(self):
config = self.config_parser
options = self.arguments
logger.debug("Config: {0}".format(config))
# check the ignore_cache setting: first from command line,
# if not present check from ini file
self.ignore_cache = False
if options.ignore_cache:
self.ignore_cache = options.ignore_cache
elif config.has_option('jenkins', 'ignore_cache'):
if config.has_option('jenkins', 'ignore_cache'):
logging.warn('''ignore_cache option should be moved to the
[job_builder] section in the config file, the one
specified in the [jenkins] section will be ignored in
@ -185,21 +174,39 @@ class JJBConfig(object):
# catching 'TypeError' is a workaround for python 2.6 interpolation
# error
# https://bugs.launchpad.net/openstack-ci/+bug/1259631
if options.user:
self.user = options.user
else:
try:
self.user = config.get('jenkins', 'user')
except (TypeError, configparser.NoOptionError):
self.user = None
try:
self.user = config.get('jenkins', 'user')
except (TypeError, configparser.NoOptionError):
pass
if options.password:
self.password = options.password
else:
try:
self.password = config.get('jenkins', 'password')
except (TypeError, configparser.NoOptionError):
self.password = None
try:
self.password = config.get('jenkins', 'password')
except (TypeError, configparser.NoOptionError):
pass
# None -- no timeout, blocking mode; same as setblocking(True)
# 0.0 -- non-blocking mode; same as setblocking(False) <--- default
# > 0 -- timeout mode; operations time out after timeout seconds
# < 0 -- illegal; raises an exception
# to retain the default must use
# "timeout=jenkins_jobs.builder._DEFAULT_TIMEOUT" or not set timeout at
# all.
try:
self.timeout = config.getfloat('jenkins', 'timeout')
except (ValueError):
raise JenkinsJobsException("Jenkins timeout config is invalid")
except (TypeError, configparser.NoOptionError):
pass
if not config.getboolean("jenkins", "query_plugins_info"):
logger.debug("Skipping plugin info retrieval")
self.plugins_info = []
self.recursive = config.getboolean('job_builder', 'recursive')
self.excludes = config.get('job_builder', 'exclude').split(os.pathsep)
def validate(self):
config = self.config_parser
# Inform the user as to what is likely to happen, as they may specify
# a real jenkins instance in test mode to get the plugin info to check
@ -213,67 +220,12 @@ class JJBConfig(object):
"Password provided, please check your configuration."
)
# None -- no timeout, blocking mode; same as setblocking(True)
# 0.0 -- non-blocking mode; same as setblocking(False) <--- default
# > 0 -- timeout mode; operations time out after timeout seconds
# < 0 -- illegal; raises an exception
# to retain the default must use
# "timeout=jenkins_jobs.builder._DEFAULT_TIMEOUT" or not set timeout at
# all.
self.timeout = builder._DEFAULT_TIMEOUT
try:
self.timeout = config.getfloat('jenkins', 'timeout')
except (ValueError):
raise JenkinsJobsException("Jenkins timeout config is invalid")
except (TypeError, configparser.NoOptionError):
pass
if (self.plugins_info is not None and
not isinstance(self.plugins_info, list)):
raise JenkinsJobsException("plugins_info must contain a list!")
self.plugins_info = None
if getattr(options, 'plugins_info_path', None) is not None:
with io.open(options.plugins_info_path, 'r',
encoding='utf-8') as yaml_file:
self.plugins_info = yaml.load(yaml_file)
if not isinstance(self.plugins_info, list):
raise JenkinsJobsException("{0} must contain a Yaml list!"
.format(options.plugins_info_path))
elif (not options.conf or not
config.getboolean("jenkins", "query_plugins_info")):
logger.debug("Skipping plugin info retrieval")
self.plugins_info = {}
if options.allow_empty_variables is not None:
# Temporary until yamlparser is refactored to query config object
if self.allow_empty_variables is not None:
config.set('job_builder',
'allow_empty_variables',
str(options.allow_empty_variables))
if getattr(options, 'path', None):
if hasattr(options.path, 'read'):
logger.debug("Input file is stdin")
if options.path.isatty():
if platform.system() == 'Windows':
key = 'CTRL+Z'
else:
key = 'CTRL+D'
logger.warn("""Reading configuration from STDIN. Press %s
to end input.""", key)
else:
# take list of paths
options.path = options.path.split(os.pathsep)
do_recurse = (getattr(options, 'recursive', False) or
config.getboolean('job_builder', 'recursive'))
excludes = [e for elist in options.exclude
for e in elist.split(os.pathsep)] or \
config.get('job_builder', 'exclude').split(os.pathsep)
paths = []
for path in options.path:
if do_recurse and os.path.isdir(path):
paths.extend(utils.recurse_path(path, excludes))
else:
paths.append(path)
options.path = paths
self.config_parser = config
self.arguments = options
str(self.allow_empty_variables))

@ -0,0 +1,7 @@
[jenkins]
user=jenkins_user
password=jenkins_password
[job_builder]
allow_empty_variables=True
ignore_cache=True

@ -7,10 +7,10 @@ import tempfile
import yaml
import jenkins
from six.moves import StringIO
import testtools
from jenkins_jobs.cli import entry
from jenkins_jobs.errors import JenkinsJobsException
from tests.base import mock
from tests.cmd.test_cmd import CmdTestsBase
@ -151,10 +151,11 @@ class TestTests(CmdTestsBase):
plugins_info_stub_yaml_file,
os.path.join(self.fixtures_path, 'cmd-001.yaml')]
with mock.patch('sys.stdout'):
e = self.assertRaises(JenkinsJobsException, entry.JenkinsJobs,
args)
self.assertIn("must contain a Yaml list", str(e))
stderr = StringIO()
with mock.patch('sys.stderr', stderr):
self.assertRaises(SystemExit, entry.JenkinsJobs, args)
self.assertIn("must contain a Yaml list",
stderr.getvalue())
class TestJenkinsGetPluginInfoError(CmdTestsBase):

@ -73,3 +73,33 @@ class TestConfigs(CmdTestsBase):
'non-existing.yaml']
jenkins_jobs = entry.JenkinsJobs(args)
self.assertRaises(IOError, jenkins_jobs.execute)
def test_config_options_not_replaced_by_cli_defaults(self):
"""
Run test mode and check config settings from conf file retained
when non of the global CLI options are set.
"""
config_file = os.path.join(self.fixtures_path,
'settings_from_config.ini')
args = ['--conf', config_file, 'test', 'dummy.yaml']
jenkins_jobs = entry.JenkinsJobs(args)
self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True)
self.assertEqual(jenkins_jobs.jjb_config.user, "jenkins_user")
self.assertEqual(jenkins_jobs.jjb_config.password, "jenkins_password")
self.assertEqual(jenkins_jobs.jjb_config.config_parser.get(
'job_builder', 'allow_empty_variables'), "True")
def test_config_options_overriden_by_cli(self):
"""
Run test mode and check config settings from conf file retained
when non of the global CLI options are set.
"""
args = ['--user', 'myuser', '--password', 'mypassword',
'--ignore-cache', '--allow-empty-variables',
'test', 'dummy.yaml']
jenkins_jobs = entry.JenkinsJobs(args)
self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True)
self.assertEqual(jenkins_jobs.jjb_config.user, "myuser")
self.assertEqual(jenkins_jobs.jjb_config.password, "mypassword")
self.assertEqual(jenkins_jobs.jjb_config.config_parser.get(
'job_builder', 'allow_empty_variables'), "True")