Skip to content

Commit f7370eb

Browse files
committed
chore(github): address Adam Thornton's review comments
1 parent 2c0d562 commit f7370eb

File tree

3 files changed

+16
-7
lines changed

3 files changed

+16
-7
lines changed

docs/source/auth-providers.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,13 @@ The GitHub repository permissions are mapped to [Giftless permissions](#permissi
205205
To minimize the traffic to GitHub for each LFS action, most of the auth data is being temporarily cached in memory, which improves performance, but naturally also ignores immediate changes for identities with changed permissions.
206206

207207
### GitHub Auth Flow
208-
Here's a description of the authentication & authorization flow. If any of these steps fails, the request gets rejected. As the supported token flavors have a very different way of authentication, they're described separately:
208+
Here's a description of the authentication & authorization flow. If any of these steps fails, the request gets rejected. As the supported token flavors have very different ways of authentication, they're described separately:
209209

210210
#### Personal Access Tokens (`ghp_`, `_github_pat_` and likely other [token flavors](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-authentication-to-github#githubs-token-formats) `gho_`, `ghu_`)
211211
These tokens eventually represent a real user. For the authenticator to work properly, the token must have these permissions:
212212
- `read:org` for "Classic" or
213213
- `metadata:read` for the fine-grained kind.
214-
- The user has to be a collaborator of the target repository with an adequate role for reading or writing.
214+
- The user has to be a collaborator on the target repository with an adequate role for reading or writing.
215215

216216
1. The URI of the primary git LFS (HTTP) [`batch` request](https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md) is used to determine what GitHub organization and repository is being targeted (e.g. `https://<server>/<org>/<repo>.git/info/lfs/...`). The request's `Authentication` header is searched for the required token in the `password` part of the `Basic` HTTP auth.
217217
2. The token is then used in a [`/user`](https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-the-authenticated-user) GitHub API call to get its identity data.

giftless/auth/github.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ def _extract_auth(self, request: flask.Request) -> tuple[str | None, str]:
350350
def api_get(self, uri: str) -> dict[str, Any]:
351351
if self._session is None:
352352
raise RuntimeError(
353-
"Calling CallContext.api_get() outside of a with block."
353+
"CallContext is a context manager maintaining a requests "
354+
"session. Call api_get() only within its entered context."
354355
)
355356
response = self._session.get(
356357
f"{self._api_url}{uri}",
@@ -365,8 +366,9 @@ def api_get_paginated(
365366
) -> Generator[dict[str, Any], None, None]:
366367
if self._session is None:
367368
raise RuntimeError(
368-
"Calling CallContext.api_get_paginated() "
369-
"outside of a with block."
369+
"CallContext is a context manager maintaining a requests "
370+
"session. Call api_get_paginated() only within its entered "
371+
"context."
370372
)
371373

372374
per_page = min(max(per_page, 1), 100)
@@ -384,7 +386,13 @@ def api_get_paginated(
384386

385387
yield from (item for item in response_json.get(list_name, []))
386388

389+
# check the 'link' header for the 'next' page URL
387390
if next_url := response.links.get("next", {}).get("url"):
391+
# extract the page number from the URL that looks like
392+
# https://api.github.com/some/collections?page=4
393+
# urlparse(next_url).query returns "page=4"
394+
# parse_qs() parses that into {'page': ['4']}
395+
# when 'page' is missing, we supply a fake ['0'] to stop
388396
next_page = int(
389397
parse_qs(urlparse(next_url).query).get("page", ["0"])[0]
390398
)
@@ -424,7 +432,8 @@ def _perm_ttl(perms: set[Permission]) -> float:
424432
else:
425433
return cc.auth_other_ttl
426434

427-
# expiration factory providing a 'ttu' function respecting least_ttl
435+
# expiration factory providing a 'ttu' function for 'TLRUCache'
436+
# respecting specified least_ttl
428437
def expiration(
429438
least_ttl: float | None = None,
430439
) -> Callable[[Any, set[Permission], float], float]:

tests/auth/test_github.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ def test_config_schema_api_timeout() -> None:
190190
DEFAULT_CONFIG = gh.Config.from_dict({})
191191
DEFAULT_USER_DICT = {
192192
"login": "kingofthebritons",
193-
"id": "12345678",
193+
"id": "125678",
194194
"name": "arthur",
195195
"email": "[email protected]",
196196
}

0 commit comments

Comments
 (0)