From 52ba1acdd667f662109c174a3ffc006d49de94b7 Mon Sep 17 00:00:00 2001 From: "Pradyun S. Gedam" Date: Wed, 21 Dec 2016 13:30:43 +0530 Subject: [PATCH 1/2] Move out all the config code to a separate module This refactor is intended to make it easier to make configuration related improvements in the future, since all the code is in a separated unit. The unit tests have been updated partially to merely update them. They now patch underscore names which means that they probably need updating. --- pip/baseparser.py | 99 ++++--------------------------- pip/configuration.py | 118 +++++++++++++++++++++++++++++++++++++ tests/unit/test_options.py | 29 ++++----- 3 files changed, 139 insertions(+), 107 deletions(-) create mode 100644 pip/configuration.py diff --git a/pip/baseparser.py b/pip/baseparser.py index 2dd4533016b..ff7c181d101 100644 --- a/pip/baseparser.py +++ b/pip/baseparser.py @@ -3,21 +3,12 @@ import sys import optparse -import os -import re import textwrap from distutils.util import strtobool from pip._vendor.six import string_types -from pip._vendor.six.moves import configparser -from pip.locations import ( - legacy_config_file, config_basename, running_under_virtualenv, - site_config_files -) -from pip.utils import appdirs, get_terminal_size - - -_environ_prefix_re = re.compile(r"^PIP_", re.I) +from pip.configuration import Configuration +from pip.utils import get_terminal_size class PrettyHelpFormatter(optparse.IndentedHelpFormatter): @@ -140,55 +131,12 @@ class ConfigOptionParser(CustomOptionParser): isolated = False def __init__(self, *args, **kwargs): - self.config = configparser.RawConfigParser() self.name = kwargs.pop('name') self.isolated = kwargs.pop("isolated", False) - self.files = self.get_config_files() - if self.files: - self.config.read(self.files) + self.config = Configuration() assert self.name optparse.OptionParser.__init__(self, *args, **kwargs) - def get_config_files(self): - # the files returned by this method will be parsed in order with the - # first files listed being overridden by later files in standard - # ConfigParser fashion - config_file = os.environ.get('PIP_CONFIG_FILE', False) - if config_file == os.devnull: - return [] - - # at the base we have any site-wide configuration - files = list(site_config_files) - - # per-user configuration next - if not self.isolated: - if config_file and os.path.exists(config_file): - files.append(config_file) - else: - # This is the legacy config file, we consider it to be a lower - # priority than the new file location. - files.append(legacy_config_file) - - # This is the new config file, we consider it to be a higher - # priority than the legacy file. - files.append( - os.path.join( - appdirs.user_config_dir("pip"), - config_basename, - ) - ) - - # finally virtualenv configuration first trumping others - if running_under_virtualenv(): - venv_config_file = os.path.join( - sys.prefix, - config_basename, - ) - if os.path.exists(venv_config_file): - files.append(venv_config_file) - - return files - def check_default(self, option, key, val): try: return option.check_value(key, val) @@ -200,26 +148,23 @@ def _update_defaults(self, defaults): """Updates the given defaults with values from the config files and the environ. Does a little special handling for certain types of options (lists).""" - # Then go and look for the other sources of configuration: - config = {} - # 1. config files - for section in ('global', self.name): - config.update( - self.normalize_keys(self.get_config_section(section)) - ) + self.config.load_config_files(self.name, self.isolated) # 2. environmental variables if not self.isolated: - config.update(self.normalize_keys(self.get_environ_vars())) + self.config.load_environment_vars() + # Accumulate complex default state. self.values = optparse.Values(self.defaults) late_eval = set() # Then set the options with those values - for key, val in config.items(): + for key, val in self.config.items(): # ignore empty values if not val: continue - option = self.get_option(key) + # '--' because configuration supports only long names + option = self.get_option('--' + key) + # Ignore options not present in this parser. E.g. non-globals put # in [global] by users that want them to apply to all applicable # commands. @@ -249,30 +194,6 @@ def _update_defaults(self, defaults): self.values = None return defaults - def normalize_keys(self, items): - """Return a config dictionary with normalized keys regardless of - whether the keys were specified in environment variables or in config - files""" - normalized = {} - for key, val in items: - key = key.replace('_', '-') - if not key.startswith('--'): - key = '--%s' % key # only prefer long opts - normalized[key] = val - return normalized - - def get_config_section(self, name): - """Get a section of a configuration""" - if self.config.has_section(name): - return self.config.items(name) - return [] - - def get_environ_vars(self): - """Returns a generator with all environmental vars with prefix PIP_""" - for key, val in os.environ.items(): - if _environ_prefix_re.search(key): - yield (_environ_prefix_re.sub("", key).lower(), val) - def get_default_values(self): """Overriding to make updating the defaults after instantiation of the option parser possible, _update_defaults() does the dirty work.""" diff --git a/pip/configuration.py b/pip/configuration.py new file mode 100644 index 00000000000..3dec31c7cf4 --- /dev/null +++ b/pip/configuration.py @@ -0,0 +1,118 @@ +"""Configuration management setup +""" + +import re +import os +import sys + +from pip._vendor.six.moves import configparser +from pip.locations import ( + legacy_config_file, config_basename, running_under_virtualenv, + site_config_files +) +from pip.utils import appdirs + + +_environ_prefix_re = re.compile(r"^PIP_", re.I) + + +class Configuration(object): + """Handles the loading of configuration files and providing an interface to + accessing data within them. + """ + + def __init__(self): + self._configparser = configparser.RawConfigParser() + self._config = {} + + def load_config_files(self, name, isolated): + """Loads configuration from configuration files + """ + files = self._get_config_files(isolated) + + if files: + self._configparser.read(files) + + for section in ('global', name): + self._config.update( + self._normalize_keys(self._get_config_section(section)) + ) + + def load_environment_vars(self): + """Loads configuration from environment variables + """ + self._config.update(self._normalize_keys(self._get_environ_vars())) + + def items(self): + """Returns key-value pairs like dict.values() representing the loaded + configuration + """ + return self._config.items() + + def _normalize_keys(self, items): + """Return a config dictionary with normalized keys regardless of + whether the keys were specified in environment variables or in config + files""" + normalized = {} + for key, val in items: + key = key.replace('_', '-') + if key.startswith('--'): + key = key[2:] # only prefer long opts + normalized[key] = val + return normalized + + def _get_environ_vars(self): + """Returns a generator with all environmental vars with prefix PIP_""" + for key, val in os.environ.items(): + if _environ_prefix_re.search(key): + yield (_environ_prefix_re.sub("", key).lower(), val) + + def _get_config_files(self, isolated): + """Returns configuration files in a defined order. + + The order is that the first files are overridden by the latter files; + like what ConfigParser expects. + """ + # the files returned by this method will be parsed in order with the + # first files listed being overridden by later files in standard + # ConfigParser fashion + config_file = os.environ.get('PIP_CONFIG_FILE', False) + if config_file == os.devnull: + return [] + + # at the base we have any site-wide configuration + files = list(site_config_files) + + # per-user configuration next + if not isolated: + if config_file and os.path.exists(config_file): + files.append(config_file) + else: + # This is the legacy config file, we consider it to be a lower + # priority than the new file location. + files.append(legacy_config_file) + + # This is the new config file, we consider it to be a higher + # priority than the legacy file. + files.append( + os.path.join( + appdirs.user_config_dir("pip"), + config_basename, + ) + ) + + # finally virtualenv configuration first trumping others + if running_under_virtualenv(): + venv_config_file = os.path.join( + sys.prefix, + config_basename, + ) + if os.path.exists(venv_config_file): + files.append(venv_config_file) + + return files + + def _get_config_section(self, section): + if self._configparser.has_section(section): + return self._configparser.items(section) + return [] \ No newline at end of file diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 16825322aaa..edf91623f1c 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -1,6 +1,6 @@ import os import pytest -import pip.baseparser +import pip.configuration from pip import main from pip import cmdoptions from pip.basecommand import Command @@ -107,8 +107,8 @@ def test_environment_override_config(self, monkeypatch): Test an environment variable overrides the config file """ monkeypatch.setattr( - pip.baseparser.ConfigOptionParser, - "get_config_section", + pip.configuration.Configuration, + "_get_config_section", self.get_config_section, ) os.environ['PIP_TIMEOUT'] = '-1' @@ -120,8 +120,8 @@ def test_commmand_config_override_global_config(self, monkeypatch): Test that command config overrides global config """ monkeypatch.setattr( - pip.baseparser.ConfigOptionParser, - "get_config_section", + pip.configuration.Configuration, + "_get_config_section", self.get_config_section, ) options, args = main(['fake']) @@ -132,8 +132,8 @@ def test_global_config_is_used(self, monkeypatch): Test that global config is used """ monkeypatch.setattr( - pip.baseparser.ConfigOptionParser, - "get_config_section", + pip.configuration.Configuration, + "_get_config_section", self.get_config_section_global, ) options, args = main(['fake']) @@ -265,23 +265,16 @@ def test_client_cert(self): class TestOptionsConfigFiles(object): def test_venv_config_file_found(self, monkeypatch): - # We only want a dummy object to call the get_config_files method - monkeypatch.setattr( - pip.baseparser.ConfigOptionParser, - '__init__', - lambda self: None, - ) - # strict limit on the site_config_files list - monkeypatch.setattr(pip.baseparser, 'site_config_files', ['/a/place']) + monkeypatch.setattr(pip.configuration, 'site_config_files', ['/a/place']) # If we are running in a virtualenv and all files appear to exist, # we should see two config files. monkeypatch.setattr( - pip.baseparser, + pip.configuration, 'running_under_virtualenv', lambda: True, ) monkeypatch.setattr(os.path, 'exists', lambda filename: True) - cp = pip.baseparser.ConfigOptionParser() - assert len(cp.get_config_files()) == 4 + cp = pip.configuration.Configuration() + assert len(cp._get_config_files(isolated=False)) == 4 From 0de88294d5ccbfa7293c24577712c089a11cdae2 Mon Sep 17 00:00:00 2001 From: "Pradyun S. Gedam" Date: Thu, 22 Dec 2016 12:07:18 +0530 Subject: [PATCH 2/2] Fix pep8 errors --- pip/configuration.py | 2 +- tests/unit/test_options.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pip/configuration.py b/pip/configuration.py index 3dec31c7cf4..1978cbcadd0 100644 --- a/pip/configuration.py +++ b/pip/configuration.py @@ -115,4 +115,4 @@ def _get_config_files(self, isolated): def _get_config_section(self, section): if self._configparser.has_section(section): return self._configparser.items(section) - return [] \ No newline at end of file + return [] diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index edf91623f1c..e93482767b3 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -266,7 +266,9 @@ class TestOptionsConfigFiles(object): def test_venv_config_file_found(self, monkeypatch): # strict limit on the site_config_files list - monkeypatch.setattr(pip.configuration, 'site_config_files', ['/a/place']) + monkeypatch.setattr( + pip.configuration, 'site_config_files', ['/a/place'] + ) # If we are running in a virtualenv and all files appear to exist, # we should see two config files.