From b0d0bde630087cc0d68085cd3c58cc2d5891f799 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 27 Aug 2018 16:11:59 -0600 Subject: [PATCH 1/5] Don't print a traceback on KeyboardInterrupt --- doctr/__main__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doctr/__main__.py b/doctr/__main__.py index 86bcbe93..c305a104 100644 --- a/doctr/__main__.py +++ b/doctr/__main__.py @@ -249,6 +249,8 @@ def process_args(parser): return args.func(args, parser) except RuntimeError as e: sys.exit("Error: " + e.args[0]) + except KeyboardInterrupt: + sys.exit(red("Interrupted by user")) def on_travis(): return os.environ.get("TRAVIS_JOB_NUMBER", '') From e3dc252840d6802f667f06377c2fd98db91fbbc8 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 27 Aug 2018 16:12:32 -0600 Subject: [PATCH 2/5] Better messaging on 401 and 403 errors from GitHub 401 errors are caused when the OTP expires from a long session. The way the code is currently organized, it isn't easy to re-ask for it, so for now we just print a message to try again. This generally only happens if the user does something in the middle of running doctr configure, such as going to enable Travis on the repo. If everything is already configured, a single session is generally short enough to use the same OTP code. 403 errors occur when the GitHub API rate limit is hit. This can happen when unauthenticated requests are used (i.e., --no-upload-key), as the limit is 60 global GitHub API requests per IP per hour. For authenticated requests, the limit is 5000 requests per hour, but this is shared across all oauth applications. It seems that the Travis "sync account" button consistently causes this limit to be hit if you have access to many repos (for instance, if you are a member of the conda-forge organization). So if a user goes to enable a repo on Travis, then runs doctr configure, they will hit this error. doctr configure now prints an error message indicating that the rate limit has been hit and how long it will be until it resets. Unfortunately, there is not much else we can do here. Fixes #311. --- doctr/__main__.py | 6 +++- doctr/local.py | 72 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/doctr/__main__.py b/doctr/__main__.py index c305a104..76a623fb 100644 --- a/doctr/__main__.py +++ b/doctr/__main__.py @@ -36,7 +36,7 @@ from .local import (generate_GitHub_token, encrypt_variable, encrypt_to_file, upload_GitHub_deploy_key, generate_ssh_key, check_repo_exists, - GitHub_login, guess_github_repo, AuthenticationFailed) + GitHub_login, guess_github_repo, AuthenticationFailed, GitHubError) from .travis import (setup_GitHub_push, commit_docs, push_docs, get_current_repo, sync_from_log, find_sphinx_build_dir, run, get_travis_branch, copy_to_tmp, checkout_deploy_branch) @@ -401,6 +401,8 @@ def configure(args, parser): is_private = check_repo_exists(build_repo, service='github', **login_kwargs) check_repo_exists(build_repo, service='travis') get_build_repo = True + except GitHubError: + raise except RuntimeError as e: print(red('\n{!s:-^{}}\n'.format(e, 70))) @@ -415,6 +417,8 @@ def configure(args, parser): check_repo_exists(deploy_repo, service='github', **login_kwargs) get_deploy_repo = True + except GitHubError: + raise except RuntimeError as e: print(red('\n{!s:-^{}}\n'.format(e, 70))) diff --git a/doctr/local.py b/doctr/local.py index 9a6b1fdb..5a488fb6 100644 --- a/doctr/local.py +++ b/doctr/local.py @@ -9,6 +9,8 @@ import re from getpass import getpass import urllib +import datetime +import textwrap import requests from requests.auth import HTTPBasicAuth @@ -130,11 +132,13 @@ def GitHub_login(*, username=None, password=None, OTP=None, headers=None): if two_factor: if OTP: print(red("Invalid authentication code")) + # For SMS, we have to make a fake request (that will fail without + # the OTP) to get GitHub to send it. See https://github.com/drdoctr/doctr/pull/203 auth_header = base64.urlsafe_b64encode(bytes(username + ':' + password, 'utf8')).decode() login_kwargs = {'auth': None, 'headers': {'Authorization': 'Basic {}'.format(auth_header)}} try: generate_GitHub_token(**login_kwargs) - except requests.exceptions.HTTPError: + except (requests.exceptions.HTTPError, GitHubError): pass print("A two-factor authentication code is required:", two_factor.split(';')[1].strip()) OTP = input("Authentication code: ") @@ -142,10 +146,64 @@ def GitHub_login(*, username=None, password=None, OTP=None, headers=None): raise AuthenticationFailed("invalid username or password") - r.raise_for_status() + GitHub_raise_for_status(r) return {'auth': auth, 'headers': headers} +class GitHubError(RuntimeError): + pass + +def GitHub_raise_for_status(r): + """ + Call instead of r.raise_for_status() for GitHub requests + + Checks for common GitHub response issues and prints messages for them. + """ + # This will happen if the doctr session has been running too long and the + # OTP code gathered from GitHub_login has expired. + + # TODO: Refactor the code to re-request the OTP without exiting. + if r.status_code == 401 and r.headers.get('X-GitHub-OTP'): + raise GitHubError("The two-factor authentication code has expired. Please run doctr configure again.") + if r.status_code == 403 and r.headers.get('X-RateLimit-Remaining') == '0': + reset = int(r.headers['X-RateLimit-Reset']) + limit = int(r.headers['X-RateLimit-Limit']) + reset_datetime = datetime.datetime.fromtimestamp(reset, datetime.timezone.utc) + relative_reset_datetime = reset_datetime - datetime.datetime.now(datetime.timezone.utc) + # Based on datetime.timedelta.__str__ + mm, ss = divmod(relative_reset_datetime.seconds, 60) + hh, mm = divmod(mm, 60) + def plural(n): + return n, abs(n) != 1 and "s" or "" + + s = "%02d minute%s" % plural(mm) + if hh: + s = "%d hour%s, " % plural(hh) + s + if relative_reset_datetime.days: + s = ("%d day%s, " % plural(relative_reset_datetime.days)) + s + authenticated = limit >= 100 + message = """\ +Your GitHub API rate limit has been hit. GitHub allows {limit} {un}authenticated +requests per hour. See {documentation_url} +for more information. +""".format(limit=limit, un="" if authenticated else "un", documentation_url=r.json()["documentation_url"]) + if authenticated: + message += """ +Note that GitHub's API limits are shared across all oauth applications. A +common cause of hitting the rate limit is the Travis "sync account" button. +""" + else: + message += """ +You can get a higher API limit by authenticating. Try running doctr configure +again without the --no-upload-key flag. +""" + message += """ +Your rate limits will reset in {s}.\ +""".format(s=s) + raise GitHubError(message) + r.raise_for_status() + + def GitHub_post(data, url, *, auth, headers): """ POST the data ``data`` to GitHub. @@ -154,7 +212,7 @@ def GitHub_post(data, url, *, auth, headers): """ r = requests.post(url, auth=auth, headers=headers, data=json.dumps(data)) - r.raise_for_status() + GitHub_raise_for_status(r) return r.json() @@ -185,7 +243,7 @@ def generate_GitHub_token(*, note="Doctr token for pushing to gh-pages from Trav def delete_GitHub_token(token_id, *, auth, headers): """Delete a temporary GitHub token""" r = requests.delete('https://api.github.com/authorizations/{id}'.format(id=token_id), auth=auth, headers=headers) - r.raise_for_status() + GitHub_raise_for_status(r) def upload_GitHub_deploy_key(deploy_repo, ssh_key, *, read_only=False, @@ -265,7 +323,11 @@ def check_repo_exists(deploy_repo, service='github', *, auth=None, headers=None) repo=repo, service=service)) - r.raise_for_status() + if service == 'github': + GitHub_raise_for_status(r) + else: + r.raise_for_status() + private = r.json().get('private', False) if wiki and not private: From 40545d2cb7f028bc5997d4cb1168fd08adeb97c7 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 27 Aug 2018 16:22:18 -0600 Subject: [PATCH 3/5] Don't print the minutes with a leading 0 --- doctr/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doctr/local.py b/doctr/local.py index 5a488fb6..03a25173 100644 --- a/doctr/local.py +++ b/doctr/local.py @@ -176,7 +176,7 @@ def GitHub_raise_for_status(r): def plural(n): return n, abs(n) != 1 and "s" or "" - s = "%02d minute%s" % plural(mm) + s = "%d minute%s" % plural(mm) if hh: s = "%d hour%s, " % plural(hh) + s if relative_reset_datetime.days: From 2696f6f608d626ae86493d3242bc153247c54408 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Mon, 27 Aug 2018 16:25:00 -0600 Subject: [PATCH 4/5] Remove unused import --- doctr/local.py | 1 - 1 file changed, 1 deletion(-) diff --git a/doctr/local.py b/doctr/local.py index 03a25173..d9e86f04 100644 --- a/doctr/local.py +++ b/doctr/local.py @@ -10,7 +10,6 @@ from getpass import getpass import urllib import datetime -import textwrap import requests from requests.auth import HTTPBasicAuth From cc7ece0840cd24c39d2388737fd8bf5336cc7f03 Mon Sep 17 00:00:00 2001 From: Aaron Meurer Date: Wed, 19 Sep 2018 11:10:41 -0600 Subject: [PATCH 5/5] Always print the exit error messages in red --- doctr/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doctr/__main__.py b/doctr/__main__.py index 76a623fb..2e6633ef 100644 --- a/doctr/__main__.py +++ b/doctr/__main__.py @@ -248,7 +248,7 @@ def process_args(parser): try: return args.func(args, parser) except RuntimeError as e: - sys.exit("Error: " + e.args[0]) + sys.exit(red("Error: " + e.args[0])) except KeyboardInterrupt: sys.exit(red("Interrupted by user"))