[gerrit_util] Cache resolved Authenticator as a class variable.

I noticed that the Authenticator is resolved maybe 5 or 6 times per
git-cl invocation. This should lead to more consistent behavior and
will likely be a bit faster, especially for SSOAuthenticator and
LuciAuthAuthenticator which involve subprocess invocations.

R=ayatane@chromium.org

Bug: 336351842
Change-Id: Id6c2873a6960a171305560acb98afe2c4f397295
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5589865
Auto-Submit: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
changes/65/5589865/18
Robert Iannucci 8 months ago committed by LUCI CQ
parent bdf64705c3
commit 137bb69871

@ -174,6 +174,10 @@ ssoHelper = SSOHelper()
class Authenticator(object):
"""Base authenticator class for authenticator implementations to subclass."""
# Cached Authenticator subclass instance, resolved via get().
_resolved: Optional[Authenticator] = None
_resolved_lock = threading.Lock()
def authenticate(self, conn: HttpConn):
"""Adds authentication information to the HttpConn."""
raise NotImplementedError()
@ -193,47 +197,54 @@ class Authenticator(object):
environment."""
raise NotImplementedError()
@staticmethod
def get():
@classmethod
def get(cls):
"""Returns: (Authenticator) The identified Authenticator to use.
Probes the local system and its environment and identifies the
Authenticator instance to use.
The resolved Authenticator instance is cached as a class variable.
"""
use_new_auth = newauth.Enabled()
# Allow skipping SSOAuthenticator for local testing purposes.
skip_sso = newauth.SkipSSO()
authenticators: List[Type[Authenticator]]
if use_new_auth:
LOGGER.debug('Authenticator.get: using new auth stack.')
authenticators = [
SSOAuthenticator,
LuciContextAuthenticator,
GceAuthenticator,
LuciAuthAuthenticator,
]
if skip_sso:
LOGGER.debug('Authenticator.get: skipping SSOAuthenticator.')
authenticators = authenticators[1:]
else:
authenticators = [
LuciContextAuthenticator,
GceAuthenticator,
CookiesAuthenticator,
]
for candidate in authenticators:
if candidate.is_applicable():
LOGGER.debug('Authenticator.get: Selected %s.',
candidate.__name__)
return candidate()
auth_names = ', '.join(a.__name__ for a in authenticators)
raise ValueError(
f"Could not find suitable authenticator, tried: [{auth_names}].")
with cls._resolved_lock:
if ret := cls._resolved:
return ret
use_new_auth = newauth.Enabled()
# Allow skipping SSOAuthenticator for local testing purposes.
skip_sso = newauth.SkipSSO()
if use_new_auth:
LOGGER.debug('Authenticator.get: using new auth stack.')
authenticators = [
SSOAuthenticator,
LuciContextAuthenticator,
GceAuthenticator,
LuciAuthAuthenticator,
]
if skip_sso:
LOGGER.debug('Authenticator.get: skipping SSOAuthenticator.')
authenticators = authenticators[1:]
else:
authenticators = [
LuciContextAuthenticator,
GceAuthenticator,
CookiesAuthenticator,
]
for candidate in authenticators:
if candidate.is_applicable():
LOGGER.debug('Authenticator.get: Selected %s.',
candidate.__name__)
ret = candidate()
cls._resolved = ret
return ret
auth_names = ', '.join(a.__name__ for a in authenticators)
raise ValueError(
f"Could not find suitable authenticator, tried: [{auth_names}]."
)
class SSOAuthenticator(Authenticator):

@ -669,6 +669,7 @@ class TestGitCl(unittest.TestCase):
# It's important to reset settings to not have inter-tests interference.
git_cl.settings = git_cl.Settings()
self.addCleanup(mock.patch.stopall)
gerrit_util.Authenticator._resolved = None
def tearDown(self):
try:
@ -3623,6 +3624,7 @@ class ChangelistTest(unittest.TestCase):
self.mockGit = GitMocks()
mock.patch('scm.GIT.GetConfig', self.mockGit.GetConfig).start()
mock.patch('scm.GIT.SetConfig', self.mockGit.SetConfig).start()
gerrit_util.Authenticator._resolved = None
def testRunHook(self):
expected_results = {

Loading…
Cancel
Save