Skip to content

Refactor proactive refresh logic for better reuse #12601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chlowell opened this issue Jul 17, 2020 · 1 comment · Fixed by #17722
Closed

Refactor proactive refresh logic for better reuse #12601

chlowell opened this issue Jul 17, 2020 · 1 comment · Fixed by #17722
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@chlowell
Copy link
Member

This logic:

if not token:
  # get new token
elif should_refrsesh:
  try:
     # get new token
  except Exception:
     # swallow

seems to be present in most if not all the credentials. Perhaps it could be moved into a base or mixin, and have the implementation just provide a callback or an override for the # get new token functionality?

Originally posted by @schaabs in #12136

That comment thread also has a sketch of a potential solution:

class CredentialBase(ABC):
    def __init__(self, **kwargs):
        self._client = AadClient(...)

    def _get_token_impl(self, *scopes, **kwargs):
        if not scopes:
            raise ValueError('"get_token" requires at least one scope')

        token = self._client.get_cached_access_token(scopes)
        if not token:
            token = self._request_token(scopes, **kwargs)
        elif self._client.should_refresh(token):
            try:
                self._request_token(scopes, **kwargs)
            except Exception:  # pylint:disable=broad-except
                pass
        return token

    @abc.abstractmethod
    def _request_token(self, *scopes, **kwargs):
        pass

class Credential(CredentialBase):
    def get_token(*scopes, **kwargs):
        """user-facing docstring"""
        return self._get_token_impl(*scopes, **kwargs)

    def _request_token(*scopes, **kwargs):
        """get a new token according to this credential's personal idiom"""
        ...
@chlowell chlowell added Azure.Identity Client This issue points to a problem in the data-plane of the library. labels Jul 17, 2020
@chlowell chlowell added this to the Backlog milestone Jul 17, 2020
@chlowell
Copy link
Member Author

#13053 added GetTokenMixin as a general solution. The remaining work here is refactoring credentials to use it.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant