fix: add type hints to credentials #1605
Conversation
|
|
||
|
|
||
| class _BaseCredentials(metaclass=abc.ABCMeta): | ||
| class BaseCredentials(metaclass=abc.ABCMeta): |
There was a problem hiding this comment.
Why do we want to change this to a public class?
There was a problem hiding this comment.
_BaseCredentials is imported from outside the _credentials_base.py file, so it's not private in that scope. You can see that it's captured as an error by running:
pyright google/auth/credentials.py| raise NotImplementedError("Refresh must be implemented") | ||
|
|
||
| def _apply(self, headers, token=None): | ||
| def _apply(self, headers: dict[str, str], token: Optional[str] = None): |
There was a problem hiding this comment.
Shall we use Mapping from typing (here and in other places)? Also update the docstring to Mapping[str, str].
There was a problem hiding this comment.
This operation requires the type of headers has __setitem__ method that's why being a Mapping is not enough:
headers["authorization"] = "Bearer {}".format(
_helpers.from_bytes(token or self.token)
)There was a problem hiding this comment.
Do you want me to modify the doctring instead?
google/auth/_refresh_worker.py
Outdated
| self._lock = threading.Lock() # protects access to worker threads. | ||
|
|
||
| def start_refresh(self, cred, request): | ||
| def start_refresh(self, cred: Credentials, request: Request): |
There was a problem hiding this comment.
| def start_refresh(self, cred: Credentials, request: Request): | |
| def start_refresh(self, cred: Credentials, request: Request): -> bool |
google/auth/_refresh_worker.py
Outdated
| self._worker.start() | ||
| return True | ||
|
|
||
| def clear_error(self): |
There was a problem hiding this comment.
| def clear_error(self): | |
| def clear_error(self): -> None |
google/auth/credentials.py
Outdated
| from google.auth._credentials_base import _BaseCredentials | ||
| from google.auth._credentials_base import BaseCredentials | ||
| from google.auth._refresh_worker import RefreshThreadManager | ||
| from google.auth.credentials import Credentials |
There was a problem hiding this comment.
We're importing from the file itself? This doesn't seem right.
There was a problem hiding this comment.
| from google.auth.credentials import Credentials |
There was a problem hiding this comment.
Ah, yeah, sorry you're right
parthea
changed the title
2 participants
Enables googleapis/python-cloud-core#302
I checked:
pyright google/auth/_credentials_base.pypyright google/auth/_default.pypyright google/auth/_refresh_worker.pypyright google/auth/aio/credentials.pypyright google/auth/credentials.pypyright google/auth/metrics.pypyright google/oauth2/service_account.py