Skip to content

Commit

Permalink
The /user/repos API requires authentication
Browse files Browse the repository at this point in the history
  • Loading branch information
@mgedmin
mgedmin committed Jul 8, 2022
1 parent de47740 commit 5048af6
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Changelog

- Add support for Python 3.10.
- Drop support for Python 3.6.
- Fix ``ghcloneall --user ... --github-token ... --include-private`` not
including any private repositories (GH: #16).


1.10.1 (2021-05-26)
Expand Down
3 changes: 2 additions & 1 deletion ghcloneall.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ def __init__(self, dry_run=False, verbose=0, progress=None, quiet=False,
self.lock = threading.Lock()

self.session = requests.Session()
self.has_auth_token = bool(token)
if token:
self.session.auth = ('', token)

Expand Down Expand Up @@ -441,7 +442,7 @@ def list_repos(self, user=None, organization=None, pattern=None,
owner)
elif user and not organization:
owner = user
if include_private:
if include_private and self.has_auth_token:

This comment has been minimized.

Sign in to view

Copy link
@dhellmann

dhellmann Jul 8, 2022

Contributor

If include_private is set and there is no token, there will not be any private repos in the output. Maybe that's better than just defaulting include_private to false? It may warrant some docs.

This comment has been minimized.

Sign in to view

Copy link
@mgedmin

mgedmin Jul 9, 2022

Author Owner

I think it would make the most sense to default include_private to True iff the user provides a GH API token.

This is essentially current behavior: if you don't provide a token, the GH API won't return any private repos (AFAIU), so the client-side filtering done by --include/exclude-private is moot. (OTOH maybe you want to use auth to avoid GH rate limiting for some reason, but don't need private repos, so the option could still be useful.)

Actually, I should double-check whether the GH API returns private repos for /orgs/{name}/repos. It could be that that option was always pointless. IIRC I was interested in ignoring forks/archived repos, and then went to look at the schema of the returned JSON for things that make sense to filter on, then added command-line options for all off them without much thought.

This comment has been minimized.

Sign in to view

Copy link
@dhellmann

dhellmann Jul 9, 2022

Contributor

See what you think of #17
# users/$name/repos does not include private repos, so
# we have to query for the repos owned by the current
# user instead. This only works if the current token
Expand Down
24 changes: 12 additions & 12 deletions tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def Repo(name, **kwargs):

def test_RepoWrangler_list_repos_for_user(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/test_user/repos',
pages=[
[
repo('xyzzy'),
Expand Down Expand Up @@ -798,7 +798,7 @@ def test_RepoWrangler_list_repos_for_org(mock_requests_get):

def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/test_user/repos',
pages=[
[
repo('xyzzy'),
Expand All @@ -815,7 +815,7 @@ def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):

def test_RepoWrangler_list_repos_filter_by_status(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/test_user/repos',
pages=[
[
repo('a', archived=True),
Expand Down Expand Up @@ -901,7 +901,7 @@ def test_RepoWrangler_list_repos_no_private(mock_requests_get):

def test_RepoWrangler_list_repos_progress_bar(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/test_user/repos',
pages=[
[
repo('xyzzy'),
Expand Down Expand Up @@ -1374,22 +1374,22 @@ def test_main_no_org_gists(monkeypatch, capsys):
)


def test_main_run_error_handling(monkeypatch, capsys):
def test_main_run_error_handling_with_private_token(monkeypatch, capsys):
monkeypatch.setattr(sys, 'argv', [
'ghcloneall', '--user', 'mgedmin',
'ghcloneall', '--user', 'mgedmin', '--github-token', 'xyzzy',
])
with pytest.raises(SystemExit) as ctx:
ghcloneall.main()
assert str(ctx.value) == (
'Failed to fetch https://api.github.com/user/repos?affiliation=owner'
'&sort=full_name&per_page=100:\n'
'Failed to fetch https://api.github.com/user/repos'
'?affiliation=owner&sort=full_name&per_page=100:\n'
'not found'
)


def test_main_run_error_handling_no_private(monkeypatch, capsys):
def test_main_run_error_handling_no_private_token(monkeypatch, capsys):
monkeypatch.setattr(sys, 'argv', [
'ghcloneall', '--user', 'mgedmin', '--exclude-private',
'ghcloneall', '--user', 'mgedmin',
])
with pytest.raises(SystemExit) as ctx:
ghcloneall.main()
Expand All @@ -1405,7 +1405,7 @@ def test_main_run(monkeypatch, mock_requests_get, capsys):
'ghcloneall', '--user', 'mgedmin', '--concurrency=1',
])
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/mgedmin/repos',
pages=[
[
repo('ghcloneall'),
Expand All @@ -1428,7 +1428,7 @@ def test_main_run_start_from(monkeypatch, mock_requests_get, capsys):
'ghcloneall', '--user', 'mgedmin', '--start-from', 'x',
])
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/user/repos?affiliation=owner',
url='https://api.github.com/users/mgedmin/repos',
pages=[
[
repo('ghcloneall'),
Expand Down

0 comments on commit 5048af6

Please sign in to comment.