Merge pull request #4200 from pradyunsg/configuration-refactor-2

Reduce the API exposed by the configuration class

pip/baseparser.py

@@ -128,12 +128,12 @@ class ConfigOptionParser(CustomOptionParser):
     """Custom option parser which updates its defaults by checking the
     configuration files and environmental variables"""
 
-    isolated = False
-
     def __init__(self, *args, **kwargs):
         self.name = kwargs.pop('name')
-        self.isolated = kwargs.pop("isolated", False)
-        self.config = Configuration()
+
+        isolated = kwargs.pop("isolated", False)
+        self.config = Configuration(isolated)
+
         assert self.name
         optparse.OptionParser.__init__(self, *args, **kwargs)
 
@@ -148,10 +148,9 @@ class ConfigOptionParser(CustomOptionParser):
         """Updates the given defaults with values from the config files and
         the environ. Does a little special handling for certain types of
         options (lists)."""
-        self.config.load_config_files(self.name, self.isolated)
-        # 2. environmental variables
-        if not self.isolated:
-            self.config.load_environment_vars()
+
+        # Load the configuration
+        self.config.load(self.name)
 
         # Accumulate complex default state.
         self.values = optparse.Values(self.defaults)

pip/configuration.py

@@ -21,27 +21,17 @@ class Configuration(object):
     accessing data within them.
     """
 
-    def __init__(self):
+    def __init__(self, isolated):
         self._configparser = configparser.RawConfigParser()
         self._config = {}
+        self.isolated = isolated
 
-    def load_config_files(self, name, isolated):
-        """Loads configuration from configuration files
+    def load(self, section):
+        """Loads configuration
         """
-        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()))
+        self._load_config_files(section)
+        if not self.isolated:
+            self._load_environment_vars()
 
     def items(self):
         """Returns key-value pairs like dict.values() representing the loaded
@@ -49,6 +39,24 @@ class Configuration(object):
         """
         return self._config.items()
 
+    def _load_config_files(self, section):
+        """Loads configuration from configuration files
+        """
+        files = self._get_config_files()
+
+        if files:
+            self._configparser.read(files)
+
+        for section in ('global', section):
+            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 _normalize_keys(self, items):
         """Return a config dictionary with normalized keys regardless of
         whether the keys were specified in environment variables or in config
@@ -67,7 +75,7 @@ class Configuration(object):
             if _environ_prefix_re.search(key):
                 yield (_environ_prefix_re.sub("", key).lower(), val)
 
-    def _get_config_files(self, isolated):
+    def _get_config_files(self):
         """Returns configuration files in a defined order.
 
         The order is that the first files are overridden by the latter files;
@@ -84,7 +92,7 @@ class Configuration(object):
         files = list(site_config_files)
 
         # per-user configuration next
-        if not isolated:
+        if not self.isolated:
             if config_file and os.path.exists(config_file):
                 files.append(config_file)
             else:

tests/unit/test_options.py

@@ -278,5 +278,5 @@ class TestOptionsConfigFiles(object):
             lambda: True,
         )
         monkeypatch.setattr(os.path, 'exists', lambda filename: True)
-        cp = pip.configuration.Configuration()
-        assert len(cp._get_config_files(isolated=False)) == 4
+        cp = pip.configuration.Configuration(isolated=False)
+        assert len(cp._get_config_files()) == 4