Repository: globus/globus-sdk-python

Issue:
Creating a client with `environment="production"` does not properly translate as it would with the environment variable
The default config section has the "production" environment values.
We have handling code for this in `globus_sdk.config_get_default_environ`, but that only runs if `environment` isn't passed to a client.
As a result, config lookups are done against the `production` section and fail.

There are several ways of handling this, but one option which is probably pretty foolproof is to update this:
https://github.com/globus/globus-sdk-python/blob/d761ba9a104ba2d8a70d2e064d12ea95101596df/globus_sdk/base.py#L85-L86

Instead of doing that, try
```python
environment = config.get_default_environ(environment)
```
and update `get_default_environ` to take an input.

Discussion:

Patch:
diff --git a/globus_sdk/base.py b/globus_sdk/base.py
index ec668bcd..8fd51190 100644
--- a/globus_sdk/base.py
+++ b/globus_sdk/base.py
@@ -80,16 +80,16 @@ class BaseClient(object):
                     type(self), self.allowed_authorizer_types,
                     type(authorizer)))
 
-        # defer this default until instantiation time so that logging can
-        # capture the execution of the config load
-        if environment is None:
-            environment = config.get_default_environ()
+        # if an environment was passed, it will be used, but otherwise lookup
+        # the env var -- and in the special case of `production` translate to
+        # `default`, regardless of the source of that value
+        # logs the environment when it isn't `default`
+        self.environment = config.get_globus_environ(inputenv=environment)
 
-        self.environment = environment
         self.authorizer = authorizer
 
         if base_url is None:
-            self.base_url = config.get_service_url(environment, service)
+            self.base_url = config.get_service_url(self.environment, service)
         else:
             self.base_url = base_url
         if base_path is not None:
@@ -104,13 +104,13 @@ class BaseClient(object):
         }
 
         # verify SSL? Usually true
-        self._verify = config.get_ssl_verify(environment)
+        self._verify = config.get_ssl_verify(self.environment)
         # HTTP connection timeout
         # this is passed verbatim to `requests`, and we therefore technically
         # support a tuple for connect/read timeouts, but we don't need to
         # advertise that... Just declare it as an float value
         if http_timeout is None:
-            http_timeout = config.get_http_timeout(environment)
+            http_timeout = config.get_http_timeout(self.environment)
         self._http_timeout = http_timeout
         # handle -1 by passing None to requests
         if self._http_timeout == -1:
diff --git a/globus_sdk/config.py b/globus_sdk/config.py
index d1c7147f..1f5bc364 100644
--- a/globus_sdk/config.py
+++ b/globus_sdk/config.py
@@ -175,17 +175,25 @@ def _bool_cast(value):
     raise ValueError("Invalid config bool")
 
 
-def get_default_environ():
+def get_globus_environ(inputenv=None):
     """
-    Get the default environment to look for in the config, as a string.
+    Get the environment to look for in the config, as a string.
+
     Typically just "default", but it can be overridden with
     `GLOBUS_SDK_ENVIRONMENT` in the shell environment. In that case, any client
     which does not explicitly specify its environment will use this value.
+
+    :param inputenv: An environment which was passed, e.g. to a client
+                     instantiation
     """
-    env = os.environ.get('GLOBUS_SDK_ENVIRONMENT', 'default')
+    if inputenv is None:
+        env = os.environ.get('GLOBUS_SDK_ENVIRONMENT', 'default')
+    else:
+        env = inputenv
+
     if env == 'production':
         env = 'default'
     if env != 'default':
         logger.info(('On lookup, non-default environment: '
-                     'GLOBUS_SDK_ENVIRONMENT={}'.format(env)))
+                     'globus_environment={}'.format(env)))
     return env


Test patch:
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 7ed9106e..fee2929c 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -148,22 +148,35 @@ class ConfigParserTests(CapturedIOTestCase):
         with self.assertRaises(ValueError):
             globus_sdk.config._bool_cast("invalid")
 
-    def test_get_default_environ(self):
+    def test_get_globus_environ(self):
         """
         Confirms returns "default", or the value of GLOBUS_SDK_ENVIRONMENT
         """
-        # default if no environ value exists
-        prev_setting = None
-        if "GLOBUS_SDK_ENVIRONMENT" in os.environ:
-            prev_setting = os.environ["GLOBUS_SDK_ENVIRONMENT"]
+        # mock environ to ensure it gets reset
+        with mock.patch.dict(os.environ):
+            # set an environment value, ensure that it's returned
+            os.environ["GLOBUS_SDK_ENVIRONMENT"] = "beta"
+            self.assertEqual(globus_sdk.config.get_globus_environ(), "beta")
+
+            # clear that value, "default" should be returned
             del os.environ["GLOBUS_SDK_ENVIRONMENT"]
-        self.assertEqual(globus_sdk.config.get_default_environ(), "default")
-        # otherwise environ value
-        os.environ["GLOBUS_SDK_ENVIRONMENT"] = "beta"
-        self.assertEqual(globus_sdk.config.get_default_environ(), "beta")
-
-        # cleanup for other tests
-        if prev_setting:
-            os.environ["GLOBUS_SDK_ENVIRONMENT"] = prev_setting
-        else:
+            self.assertEqual(globus_sdk.config.get_globus_environ(), "default")
+
+            # ensure that passing a value returns that value
+            self.assertEqual(
+                globus_sdk.config.get_globus_environ("beta"), "beta")
+
+    def test_get_globus_environ_production(self):
+        """
+        Confirms that get_globus_environ translates "production" to "default",
+        including when special values are passed
+        """
+        # mock environ to ensure it gets reset
+        with mock.patch.dict(os.environ):
+            os.environ["GLOBUS_SDK_ENVIRONMENT"] = "production"
+            self.assertEqual(globus_sdk.config.get_globus_environ(), "default")
+
             del os.environ["GLOBUS_SDK_ENVIRONMENT"]
+            # ensure that passing a value returns that value
+            self.assertEqual(
+                globus_sdk.config.get_globus_environ("production"), "default")