Repository: globus-gladier/gladier (language: python)

Issue:
refactor!: Remove Globus-Automate-Client dependency
Remove the globus-automate-client dependency in favor of calling into the base globus-sdk instead


Discussion / hints:
@NickolausDS You didn't ask me to review, but I did browse through this relatively quickly and it seems pretty good to me. Overall, the idea of deprecating globus-automate-client usage seems like a good one if the automate/sdk dev team(s) are doing so as well. If nothing else, its the first I've looked at the globus sdk proper integration for flows so I learned something at least.

Thanks Jim! I might do another pass on this just to make sure there aren't any hidden failures with the Globus SDK. I didn't fully test this outside of just making sure the unit tests pass. 

Fix patch:
diff --git a/gladier/managers/login_manager.py b/gladier/managers/login_manager.py
index c1316a0..b4269b3 100644
--- a/gladier/managers/login_manager.py
+++ b/gladier/managers/login_manager.py
@@ -1,6 +1,7 @@
 import logging
 import time
 from typing import Callable, List, Set, Iterable, Union, Any, Mapping
+from typing_extensions import TypeAlias
 import abc
 
 import fair_research_login
@@ -14,6 +15,10 @@
 
 log = logging.getLogger(__name__)
 
+AUTHORIZER_MAP: TypeAlias = Mapping[
+    str, Union[AccessTokenAuthorizer, RefreshTokenAuthorizer]
+]
+
 
 class BaseLoginManager(abc.ABC):
     def __init__(self, *args, **kwargs):
@@ -26,7 +31,7 @@ def missing_authorizers(self) -> Set[str]:
 
     def get_missing_authorizers(
         self,
-        authorizers: Mapping[str, Union[AccessTokenAuthorizer, RefreshTokenAuthorizer]],
+        authorizers: AUTHORIZER_MAP,
     ) -> Set[str]:
         # Disregard any scopes not in the 'required' list. This allows implementers to return
         # unrelated scopes.
@@ -36,13 +41,22 @@ def get_missing_authorizers(
         )
         return absent | self.scope_changes
 
-    def get_manager_authorizers(self):
+    def get_manager_authorizers(self) -> AUTHORIZER_MAP:
+        """
+        Method to be called by any service manager that needs authorizers. Method will guarantee
+        live authorizers to the best ability of the current login manager. If missing authorizers
+        exist, either because of a scope change or no login has taken place, an exception will be
+        raised.
+        :returns: a dictionary of authorizers keyed by scope
+        :raises gladier.exc.AuthException: If any required scope could not be obtained
+        """
         authorizers = self.get_authorizers()
         missing = self.get_missing_authorizers(authorizers)
 
         if missing:
             log.info("Attempting login to fetch missing authorizers.")
             self.login(missing)
+            self.clear_scope_changes()
             authorizers = self.get_authorizers()
             missing = self.get_missing_authorizers(authorizers)
 
@@ -98,6 +112,13 @@ def add_scope_change(self, scopes: Iterable[str]):
         log.debug(f"Tracking scope change: {scopes}")
         self.scope_changes = self.scope_changes | set(scopes)
 
+    def clear_scope_changes(self):
+        """
+        Clear any tracked scopes that required re-authorhization. This is typically called internally
+        after successful login and should not be invoked externally.
+        """
+        self.scope_changes = set()
+
 
 class AutoLoginManager(BaseLoginManager):
     """


Test patch:
diff --git a/gladier/tests/test_flows_manager.py b/gladier/tests/test_flows_manager.py
index 1420893..ffaa484 100644
--- a/gladier/tests/test_flows_manager.py
+++ b/gladier/tests/test_flows_manager.py
@@ -113,12 +113,10 @@ def test_dependent_scope_change_run_flow(
     assert auto_login.login.call_count == 0
 
     # Gladier will re-run run_flow after login, so catch the second 'run_flow()'
-    with pytest.raises(gladier.exc.AuthException):
+    with pytest.raises(globus_sdk.GlobusAPIError):
         fm.run_flow()
-    assert auto_login.scope_changes == {
-        "https://auth.globus.org/scopes/foo/flow_foo_user"
-    }
     assert auto_login.login.call_count == 1
+    assert not auto_login.scope_changes
 
 
 def test_dependent_scope_change_no_login(logged_in, mock_flows_client, monkeypatch):
@@ -126,8 +124,9 @@ def test_dependent_scope_change_no_login(logged_in, mock_flows_client, monkeypat
     cli.login_manager.login = Mock()
     cli.login_manager.add_scope_change(["foo"])
 
-    with pytest.raises(gladier.exc.AuthException):
-        cli.run_flow()
+    cli.run_flow()
+    assert cli.login_manager.login.call_count == 1
+    assert not cli.login_manager.scope_changes
 
 
 def test_gladier_raises_globus_errors(