Repository: ansible/tower-cli
Pull request #424 (2017-10-30 20:16:00)

always read settings from environment
Connect #296 

```
$ TOWER_HOST=foo.invalid tower-cli user list
Error: There was a network error of some kind trying to connect to Tower.

The most common  reason for this is a settings issue; is your "host" value in `tower-cli config` correct?
Right now it is: "foo.invalid".
```

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13958858/badge)](https://coveralls.io/builds/13958858)

Coverage increased (+0.007%) to 90.377% when pulling **8439b502ab3e0ee6b5bc41b64376ab74a97e2278 on AlanCoding:env_always** into **c2b1fd4e1bb3c6100670868dc93af0c7c4becebf on ansible:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13958858/badge)](https://coveralls.io/builds/13958858)

Coverage increased (+0.007%) to 90.377% when pulling **8439b502ab3e0ee6b5bc41b64376ab74a97e2278 on AlanCoding:env_always** into **c2b1fd4e1bb3c6100670868dc93af0c7c4becebf on ansible:master**.

AlanCoding: One potentially small problem: this could make the behavior of test pass/fail dependent on the environment variables.

I don't want to worry about that right now, because we should be coding for the usage, not for the tests.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/14008507/badge)](https://coveralls.io/builds/14008507)

Coverage increased (+0.007%) to 90.377% when pulling **47dea705503450b933c1da495d57315d6f244a1f on AlanCoding:env_always** into **71d452452420f8241a5709c3746375dbca23ea65 on ansible:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/14008408/badge)](https://coveralls.io/builds/14008408)

Coverage increased (+0.2%) to 90.577% when pulling **36d79afefdfaab8c1121642af46b0b7e704461ed on AlanCoding:env_always** into **71d452452420f8241a5709c3746375dbca23ea65 on ansible:master**.

AlanCoding: just realized -

this will need some extra work to show the environment variables when we echo the config `tower-cli config`

Patch:
diff --git a/tower_cli/cli/misc.py b/tower_cli/cli/misc.py
index 1e8627f..5f63cb0 100644
--- a/tower_cli/cli/misc.py
+++ b/tower_cli/cli/misc.py
@@ -113,6 +113,7 @@ def config(key=None, value=None, scope='user', global_=False, unset=False):
         seen = set()
         parser_desc = {
             'runtime': 'Runtime options.',
+            'environment': 'Options from environment variables.',
             'local': 'Local options (set with `tower-cli config '
                      '--scope=local`; stored in .tower_cli.cfg of this '
                      'directory or a parent)',
diff --git a/tower_cli/conf.py b/tower_cli/conf.py
index 87901b7..1a7df2b 100644
--- a/tower_cli/conf.py
+++ b/tower_cli/conf.py
@@ -35,6 +35,11 @@ __all__ = ['settings', 'with_global_options', 'pop_option']
 tower_dir = '/etc/tower/'
 user_dir = os.path.expanduser('~')
 CONFIG_FILENAME = '.tower_cli.cfg'
+CONFIG_OPTIONS = frozenset((
+    'host', 'username', 'password', 'verify_ssl', 'format',
+    'color', 'verbose', 'description_on', 'certificate',
+    'use_token'
+))
 
 
 class Parser(configparser.ConfigParser):
@@ -83,6 +88,7 @@ class Settings(object):
         - user: Contents parsed from .ini-formatted file ``~/.tower_cli.cfg`` if exists.
         - local: Contents parsed from .ini-formatted file ``.tower_cli.cfg`` if exists in the present working
           directory or any parent directories.
+        - environment: Values from magic environment variables.
         - runtime: keyworded arguments provided by ``settings.runtime_values`` context manager.
 
     Note that .ini configuration file should follow the specified format in order to be correctly parsed:
@@ -95,7 +101,7 @@ class Settings(object):
        ...
 
     """
-    _parser_names = ['runtime', 'local', 'user', 'global', 'defaults']
+    _parser_names = ['runtime', 'environment', 'local', 'user', 'global', 'defaults']
 
     @staticmethod
     def _new_parser(defaults=None):
@@ -114,20 +120,23 @@ class Settings(object):
 
         # Initialize the data dictionary for the default level
         # precedence (that is, the bottom of the totem pole).
-        defaults = {
+        defaults = {}
+        for key in CONFIG_OPTIONS:
+            defaults[key] = ''
+        defaults.update({
             'color': 'true',
             'format': 'human',
             'host': '127.0.0.1',
-            'password': '',
-            'username': '',
             'verify_ssl': 'true',
             'verbose': 'false',
             'description_on': 'false',
-            'certificate': '',
             'use_token': 'false',
-        }
+        })
         self._defaults = self._new_parser(defaults=defaults)
 
+        # environment variables as defaults
+        self._environment = self._new_parser(defaults=config_from_environment())
+
         # If there is a global settings file, initialize it.
         self._global = self._new_parser()
         if os.path.isdir(tower_dir):
@@ -275,7 +284,6 @@ class Settings(object):
 
         =====API DOCS=====
         """
-        kwargs = config_from_environment(kwargs)
 
         # Coerce all values to strings (to be coerced back by configparser
         # later) and defenestrate any None values.
@@ -313,20 +321,16 @@ class Settings(object):
                 self._cache.pop(k, None)
 
 
-def config_from_environment(kwargs):
+def config_from_environment():
     """Read tower-cli config values from the environment if present, being
     careful not to override config values that were explicitly passed in.
     """
-    CONFIG_OPTIONS = ('host', 'username', 'password', 'verify_ssl', 'format',
-                      'color', 'verbose', 'description_on', 'certificate',
-                      'use_token')
-    kwargs = copy.copy(kwargs)
+    kwargs = {}
     for k in CONFIG_OPTIONS:
-        if k not in kwargs or kwargs[k] is None:
-            env = 'TOWER_' + k.upper()
-            v = os.getenv(env, None)
-            if v is not None:
-                kwargs[k] = v
+        env = 'TOWER_' + k.upper()
+        v = os.getenv(env, None)
+        if v is not None:
+            kwargs[k] = v
     return kwargs
 
 


Test patch:
diff --git a/tests/test_conf.py b/tests/test_conf.py
index 5cdb311..afdc878 100644
--- a/tests/test_conf.py
+++ b/tests/test_conf.py
@@ -112,25 +112,23 @@ class ConfigFromEnvironmentTests(unittest.TestCase):
     def test_no_override(self):
         """Establish that the environment variables do not override explicitly
         passed in values."""
-        settings = Settings()
         with mock.patch.dict(os.environ, {'TOWER_HOST': 'myhost'}):
+            settings = Settings()
             with settings.runtime_values(host='yourhost'):
                 self.assertEqual(settings.host, 'yourhost')
 
     def test_read_from_env(self):
         """Establish that the environment variables are correctly parsed
         and the values are set in the settings."""
-        settings = Settings()
-
         mock_env = {'TOWER_HOST': 'myhost', 'TOWER_PASSWORD': 'mypass',
                     'TOWER_USERNAME': 'myuser', 'TOWER_VERIFY_SSL': 'False'}
 
         with mock.patch.dict(os.environ, mock_env):
-            with settings.runtime_values():
-                self.assertEqual(settings.host, 'myhost')
-                self.assertEqual(settings.username, 'myuser')
-                self.assertEqual(settings.password, 'mypass')
-                self.assertEqual(settings.verify_ssl, False)
+            settings = Settings()
+            self.assertEqual(settings.host, 'myhost')
+            self.assertEqual(settings.username, 'myuser')
+            self.assertEqual(settings.password, 'mypass')
+            self.assertEqual(settings.verify_ssl, False)
 
 
 class CommandTests(unittest.TestCase):