feat: implement base classes for credentials and request sessions #1551

ohmayr · 2024-07-05T18:05:17Z

This PR implements base classes for credentials and request sessions to capture the common logic between synchronous and asynchronous code.

conventional-commit-lint-gcf · 2024-07-05T18:20:56Z

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…hub.com/googleapis/google-auth-library-python into implement-base-requests-and-credentials

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

vchudnov-g

LGTM, but get sign-off from auth team.

Quick observation: When refactoring file contents into a new file but still keeping the old file, it is helpful to reviewers if you can make the new file share the old file's previous history, so reviewers can see the diff's between the old file and the new as well as between the the old file's previous version and new version. Instructions are here: https://stackoverflow.com/a/44036771/23828460 (also see https://www.mankier.com/1/git-cp#Synopsis)

No need to do it now after the fact, but keep it in mind for future refactorings.

google/auth/transport/_requests_base.py

vchudnov-g · 2024-07-08T18:49:22Z

google/auth/_credentials_base.py

+        headers["authorization"] = "Bearer {}".format(
+            _helpers.from_bytes(token or self.token)
+        )
+        """Trust boundary value will be a cached value from global lookup.


I realize this was in the original file, but it's weird having a string floating as a comment rather than a proper Python comment.

google/auth/_credentials_base.py

google/auth/transport/requests.py

clundin25 · 2024-07-09T17:27:55Z

google/auth/transport/_requests_base.py

+_DEFAULT_TIMEOUT = 120  # in second
+
+
+class _BaseAuthorizedSession(metaclass=abc.ABCMeta):


My thoughts are similar here re:codesharing. Since the async code is a "colored" function I think it's safe to assume that the sync code and async code should never be in the same callstack.

In terms of the async semantics, yes that is a valid consideration. But there are still common parts of the logic are re-used between sync and async and those would make sense to consider factoring out. (I think in the rant you linked to, that corresponds to the "blue" functions.)

google/auth/_credentials_base.py

…hub.com/googleapis/google-auth-library-python into implement-base-requests-and-credentials

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…hub.com/googleapis/google-auth-library-python into implement-base-requests-and-credentials

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

google/auth/_credentials_base.py

parthea · 2024-07-15T19:28:05Z

nox > flake8 --import-order-style=google --application-import-names=google,tests,system_tests google tests tests_async
google/auth/credentials.py:26:1: I100 Import statements are in the wrong order. 'from google.auth._credentials_base import _BaseCredentials' should be before 'from google.auth._refresh_worker import RefreshThreadManager'
google/auth/transport/requests.py:41:1: I100 Import statements are in the wrong order. 'import google.auth.transport._mtls_helper' should be before 'from google.auth.transport._requests_base import _BaseAuthorizedSession'
nox > Command flake8 --import-order-style=google --application-import-names=google,tests,system_tests google tests tests_async failed with exit code 1
nox > Session lint failed.

…hub.com/googleapis/google-auth-library-python into implement-base-requests-and-credentials

implement base classes for credentials and request sessions

5718415

ohmayr assigned parthea Jul 5, 2024

ohmayr changed the title ~~implement base classes for credentials and request sessions~~ feat: implement base classes for credentials and request sessions Jul 5, 2024

remove public methods apply and before_request from base credentials

27ac026

ohmayr marked this pull request as ready for review July 5, 2024 19:45

ohmayr requested review from a team as code owners July 5, 2024 19:45

ohmayr added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 5, 2024

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 5, 2024

move MTLS logic back to requests

c22119b

ohmayr force-pushed the implement-base-requests-and-credentials branch from 6d0d1a4 to c22119b Compare July 5, 2024 20:18

gcf-owl-bot bot and others added 4 commits July 5, 2024 20:19

🦉 Updates from OwlBot post-processor

215ccc9

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

minor fix

dc370ef

Merge branch 'implement-base-requests-and-credentials' of https://git…

cf63305

…hub.com/googleapis/google-auth-library-python into implement-base-requests-and-credentials

🦉 Updates from OwlBot post-processor

71c093f

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

ohmayr assigned clundin25 Jul 8, 2024

vchudnov-g approved these changes Jul 8, 2024

View reviewed changes