Repository: globus/globus-sdk-python
Pull request #311 (2018-08-29 15:49:53)

fix acl for sdktester2b in test suite
Fixes the test suite error raised by transfer now checking ACLs before task submission

We could add this on creation of the shared endpoint, but unless someone feels strongly about that I'd rather not add 19 extra round trips to Transfer


Patch:
diff --git a/globus_sdk/authorizers/client_credentials.py b/globus_sdk/authorizers/client_credentials.py
index d5649942..6f82b4ee 100644
--- a/globus_sdk/authorizers/client_credentials.py
+++ b/globus_sdk/authorizers/client_credentials.py
@@ -48,16 +48,8 @@ class ClientCredentialsAuthorizer(RenewingAuthorizer):
           POSIX timestamp (i.e. seconds since the epoch)
 
         ``on_refresh`` (*callable*)
-          A callback which is triggered any time this authorizer fetches a new
-          access_token. The ``on_refresh`` callable is invoked on the
-          :class:`OAuthTokenResponse \
-                  <globus_sdk.auth.token_response.OAuthTokenResponse>`
-          object resulting from the token being refreshed.
-          It should take only one argument, the token response object.
-
-          This is useful for implementing storage for Access Tokens, as the
-          ``on_refresh`` callback can be used to update the Access Tokens and
-          their expiration times.
+          Will be called as fn(token_data) any time this authorizer
+          fetches a new access_token
     """
     def __init__(self, confidential_client, scopes,
                  access_token=None, expires_at=None, on_refresh=None):
diff --git a/globus_sdk/authorizers/refresh_token.py b/globus_sdk/authorizers/refresh_token.py
index 1cd35305..6ca6e458 100644
--- a/globus_sdk/authorizers/refresh_token.py
+++ b/globus_sdk/authorizers/refresh_token.py
@@ -43,16 +43,8 @@ class RefreshTokenAuthorizer(RenewingAuthorizer):
           POSIX timestamp (i.e. seconds since the epoch)
 
         ``on_refresh`` (*callable*)
-          A callback which is triggered any time this authorizer fetches a new
-          access_token. The ``on_refresh`` callable is invoked on the
-          :class:`OAuthTokenResponse \
-                  <globus_sdk.auth.token_response.OAuthTokenResponse>`
-          object resulting from the token being refreshed.
-          It should take only one argument, the token response object.
-
-          This is useful for implementing storage for Access Tokens, as the
-          ``on_refresh`` callback can be used to update the Access Tokens and
-          their expiration times.
+        Will be called as fn(token_data) any time this authorizer
+        fetches a new access_token
     """
     def __init__(self, refresh_token, auth_client,
                  access_token=None, expires_at=None, on_refresh=None):
diff --git a/globus_sdk/authorizers/renewing.py b/globus_sdk/authorizers/renewing.py
index e2d711e8..07fe7cd5 100644
--- a/globus_sdk/authorizers/renewing.py
+++ b/globus_sdk/authorizers/renewing.py
@@ -28,28 +28,6 @@ class RenewingAuthorizer(GlobusAuthorizer):
     To make an authorizer that implements this class implement
     the _get_token_response and _extract_token_data methods for that
     authorization type,
-
-    **Parameters**
-
-        ``access_token`` (*string*)
-          Initial Access Token to use. Used only if ``expires_at`` is also set,
-          otherwise ignored.
-
-        ``expires_at`` (*int*)
-          Expiration time for the starting ``access_token`` expressed as a
-          POSIX timestamp (i.e. seconds since the epoch)
-
-        ``on_refresh`` (*callable*)
-          A callback which is triggered any time this authorizer fetches a new
-          access_token. The ``on_refresh`` callable is invoked on the
-          :class:`OAuthTokenResponse \
-                  <globus_sdk.auth.token_response.OAuthTokenResponse>`
-          object resulting from the token being refreshed.
-          It should take only one argument, the token response object.
-
-          This is useful for implementing storage for Access Tokens, as the
-          ``on_refresh`` callback can be used to update the Access Tokens and
-          their expiration times.
     """
 
     def __init__(self, access_token=None, expires_at=None, on_refresh=None):
@@ -131,14 +109,9 @@ class RenewingAuthorizer(GlobusAuthorizer):
             self.on_refresh(res)
             logger.debug("Invoked on_refresh callback")
 
-    def check_expiration_time(self):
+    def _check_expiration_time(self):
         """
         Check if the expiration timer is done, and renew the token if it is.
-
-        This is called implicitly by ``set_authorization_header``, but you can
-        call it explicitly if you want to ensure that a token gets refreshed.
-        This can be useful in order to get at a new, valid token via the
-        ``on_refresh`` handler.
         """
         logger.debug("RenewingAuthorizer checking expiration time")
         if self.access_token is None or (
@@ -156,7 +129,7 @@ class RenewingAuthorizer(GlobusAuthorizer):
         Once that's done, sets the ``Authorization`` header to
         "Bearer <access_token>"
         """
-        self.check_expiration_time()
+        self._check_expiration_time()
         logger.debug(("Setting RefreshToken Authorization Header:"
                       'Bearer token has hash "{}"')
                      .format(self.access_token_hash))
diff --git a/globus_sdk/base.py b/globus_sdk/base.py
index 8fd51190..ec668bcd 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)))
 
-        # 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)
+        # 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()
 
+        self.environment = environment
         self.authorizer = authorizer
 
         if base_url is None:
-            self.base_url = config.get_service_url(self.environment, service)
+            self.base_url = config.get_service_url(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(self.environment)
+        self._verify = config.get_ssl_verify(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(self.environment)
+            http_timeout = config.get_http_timeout(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 1f5bc364..d1c7147f 100644
--- a/globus_sdk/config.py
+++ b/globus_sdk/config.py
@@ -175,25 +175,17 @@ def _bool_cast(value):
     raise ValueError("Invalid config bool")
 
 
-def get_globus_environ(inputenv=None):
+def get_default_environ():
     """
-    Get the environment to look for in the config, as a string.
-
+    Get the default 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
     """
-    if inputenv is None:
-        env = os.environ.get('GLOBUS_SDK_ENVIRONMENT', 'default')
-    else:
-        env = inputenv
-
+    env = os.environ.get('GLOBUS_SDK_ENVIRONMENT', 'default')
     if env == 'production':
         env = 'default'
     if env != 'default':
         logger.info(('On lookup, non-default environment: '
-                     'globus_environment={}'.format(env)))
+                     'GLOBUS_SDK_ENVIRONMENT={}'.format(env)))
     return env


Test patch:
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index fee2929c..7ed9106e 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -148,35 +148,22 @@ class ConfigParserTests(CapturedIOTestCase):
         with self.assertRaises(ValueError):
             globus_sdk.config._bool_cast("invalid")
 
-    def test_get_globus_environ(self):
+    def test_get_default_environ(self):
         """
         Confirms returns "default", or the value of 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
+        # default if no environ value exists
+        prev_setting = None
+        if "GLOBUS_SDK_ENVIRONMENT" in os.environ:
+            prev_setting = os.environ["GLOBUS_SDK_ENVIRONMENT"]
             del os.environ["GLOBUS_SDK_ENVIRONMENT"]
-            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")
-
+        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:
             del os.environ["GLOBUS_SDK_ENVIRONMENT"]
-            # ensure that passing a value returns that value
-            self.assertEqual(
-                globus_sdk.config.get_globus_environ("production"), "default")
diff --git a/tests/unit/test_renewing_authorizer.py b/tests/unit/test_renewing_authorizer.py
index ffda8551..ed227b92 100644
--- a/tests/unit/test_renewing_authorizer.py
+++ b/tests/unit/test_renewing_authorizer.py
@@ -105,7 +105,7 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         """
         Confirms nothing is done before the access_token expires,
         """
-        self.authorizer.check_expiration_time()
+        self.authorizer._check_expiration_time()
         self.assertEqual(self.authorizer.access_token, self.access_token)
 
     def test_check_expiration_time_expired(self):
@@ -113,7 +113,7 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         Confirms a new access_token is gotten after waiting for expiration
         """
         time.sleep(1)
-        self.authorizer.check_expiration_time()
+        self.authorizer._check_expiration_time()
         self.assertEqual(self.authorizer.access_token,
                          self.token_data["access_token"])
         self.assertEqual(self.authorizer.expires_at,
@@ -125,7 +125,7 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         Confirms a new access_token is gotten if the old one is set to None
         """
         self.authorizer.access_token = None
-        self.authorizer.check_expiration_time()
+        self.authorizer._check_expiration_time()
         self.assertEqual(self.authorizer.access_token,
                          self.token_data["access_token"])
         self.assertEqual(self.authorizer.expires_at,
@@ -137,7 +137,7 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         Confirms a new access_token is gotten if expires_at is set to None
         """
         self.authorizer.expires_at = None
-        self.authorizer.check_expiration_time()
+        self.authorizer._check_expiration_time()
         self.assertEqual(self.authorizer.access_token,
                          self.token_data["access_token"])
         self.assertEqual(self.authorizer.expires_at,
diff --git a/tests/unit/test_transfer_client_shared.py b/tests/unit/test_transfer_client_shared.py
index 5317b1b3..fc18d037 100644
--- a/tests/unit/test_transfer_client_shared.py
+++ b/tests/unit/test_transfer_client_shared.py
@@ -501,8 +501,12 @@ class SharedTransferClientTests(TransferClientTestCase):
         Confirms sdktester1a can see the task is paused (or about to be).
         Confirms 403 when non manager attempts to use this resource.
         """
-        # sdktester1a creates pause rule
+        # sdktester1a creates pause rule and gives sdktester2b an ACL
         rule_id = self.test_endpoint_manager_create_pause_rule()
+        acl_data = {
+            "path": "/", "permissions": "rw", "principal_type": "identity",
+            "principal": get_user_data()["sdktester2b"]["id"]}
+        self.tc.add_endpoint_acl_rule(self.test_share_ep_id, acl_data)
 
         # sdktester2b subits no-op delete task
         ddata = globus_sdk.DeleteData(self.tc2, self.test_share_ep_id,