From 1230e49bad2289401260072c06b7b54b566656fd Mon Sep 17 00:00:00 2001 From: Jose Ricardo Date: Fri, 19 Jun 2015 18:14:33 -0400 Subject: [PATCH 1/2] Raise ClickException in case of request errors Better than handling boolean returns. As it has already proved to be prone to errors (not handling the return values). --- shub/deploy.py | 10 +++------- shub/exceptions.py | 7 +++++++ shub/fetch_eggs.py | 12 ++++++++++++ shub/utils.py | 16 ++++++++++------ tests/test_fetch_eggs.py | 26 ++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 shub/exceptions.py create mode 100644 tests/test_fetch_eggs.py diff --git a/shub/deploy.py b/shub/deploy.py index 986fc6b2..db69cc5d 100644 --- a/shub/deploy.py +++ b/shub/deploy.py @@ -43,7 +43,6 @@ @click.option("--egg", help="deploy the given egg, instead of building one") @click.option("--build-egg", help="only build the given egg, don't deploy it") def cli(target, project, version, list_targets, debug, egg, build_egg): - exitcode = 0 if not inside_project(): log("Error: no Scrapy project found in this location") sys.exit(1) @@ -69,10 +68,9 @@ def cli(target, project, version, list_targets, debug, egg, build_egg): else: log("Packing version %s" % version) egg, tmpdir = _build_egg() - if _upload_egg(target, egg, project, version): - click.echo("Run your spiders at: https://dash.scrapinghub.com/p/%s/" % project) - else: - exitcode = 1 + + _upload_egg(target, egg, project, version) + click.echo("Run your spiders at: https://dash.scrapinghub.com/p/%s/" % project) if tmpdir: if debug: @@ -80,8 +78,6 @@ def cli(target, project, version, list_targets, debug, egg, build_egg): else: shutil.rmtree(tmpdir) - sys.exit(exitcode) - def _get_project(target, project): project = project or target.get('project') diff --git a/shub/exceptions.py b/shub/exceptions.py new file mode 100644 index 00000000..f0dea19c --- /dev/null +++ b/shub/exceptions.py @@ -0,0 +1,7 @@ +from click import ClickException + + +class AuthException(ClickException): + def __init__(self): + msg = 'Authentication failure. Make sure your API key is valid.' + super(AuthException, self).__init__(msg) diff --git a/shub/fetch_eggs.py b/shub/fetch_eggs.py index 6f7f4404..a4b86026 100644 --- a/shub/fetch_eggs.py +++ b/shub/fetch_eggs.py @@ -1,8 +1,10 @@ import click import requests +from click import ClickException from shub.utils import find_api_key from shub.click_utils import log +from shub.exceptions import AuthException @click.command(help="Download a project's eggs from the Scrapy Cloud") @@ -12,6 +14,8 @@ def cli(project_id): url = "https://dash.scrapinghub.com/api/eggs/bundle.zip?project=%s" % project_id rsp = requests.get(url=url, auth=auth, stream=True, timeout=300) + assert_response_is_valid(rsp) + destfile = 'eggs-%s.zip' % project_id log("Downloading eggs to %s" % destfile) @@ -20,3 +24,11 @@ def cli(project_id): if chunk: f.write(chunk) f.flush() + + +def assert_response_is_valid(rsp): + if rsp.status_code == 403: + raise AuthException() + elif rsp.status_code != 200: + msg = 'Eggs could not be fetched. Status: %d' % rsp.status_code + raise ClickException(msg) diff --git a/shub/utils.py b/shub/utils.py index 45a53140..f806cdaf 100644 --- a/shub/utils.py +++ b/shub/utils.py @@ -9,7 +9,10 @@ import requests +from click import ClickException + from shub.click_utils import log +from shub.exceptions import AuthException SCRAPY_CFG_FILE = os.path.expanduser("~/.scrapy.cfg") OS_WIN = True if os.name == 'nt' else False @@ -48,7 +51,6 @@ def get_key_netrc(): if key: return key - def make_deploy_request(url, data, files, auth): try: rsp = requests.post(url=url, auth=auth, data=data, files=files, @@ -59,12 +61,14 @@ def make_deploy_request(url, data, files, auth): return True except requests.HTTPError as exc: rsp = exc.response - log("Deploy failed ({}):".format(rsp.status_code)) - log(rsp.text) - return False + + if rsp.status_code == 403: + raise AuthException() + + msg = "Deploy failed ({}):\n{}".format(rsp.status_code, rsp.text) + raise ClickException(msg) except requests.RequestException as exc: - log("Deploy failed: {}".format(exc)) - return False + raise ClickException("Deploy failed: {}".format(exc)) def pwd_git_version(): diff --git a/tests/test_fetch_eggs.py b/tests/test_fetch_eggs.py new file mode 100644 index 00000000..804d7eae --- /dev/null +++ b/tests/test_fetch_eggs.py @@ -0,0 +1,26 @@ +import unittest +import mock +from collections import namedtuple + +from click.testing import CliRunner +from shub import tool + +FakeResponse = namedtuple('FakeResponse', ['status_code']) + +@mock.patch('shub.fetch_eggs.requests', autospec=True) +class FetchEggsTest(unittest.TestCase): + + def setUp(self): + self.runner = CliRunner() + + def test_raises_auth_exception(self, requests_mock): + fake_response = FakeResponse(403) + requests_mock.get.return_value = fake_response + output = self.runner.invoke(tool.cli, ['fetch-eggs', 'xxx']).output + self.assertTrue('Authentication failure' in output) + + def test_raises_exception_if_request_error(self, requests_mock): + fake_response = FakeResponse(400) + requests_mock.get.return_value = fake_response + output = self.runner.invoke(tool.cli, ['fetch-eggs', 'xxx']).output + self.assertTrue('Eggs could not be fetched' in output) From a005411a417f7e5d2c100f4a31090b5ef88f66d5 Mon Sep 17 00:00:00 2001 From: Jose Ricardo Date: Fri, 19 Jun 2015 18:25:07 -0400 Subject: [PATCH 2/2] Make sure `deploy` tmp files are cleaned --- shub/deploy.py | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/shub/deploy.py b/shub/deploy.py index db69cc5d..8c6b2537 100644 --- a/shub/deploy.py +++ b/shub/deploy.py @@ -54,29 +54,30 @@ def cli(target, project, version, list_targets, debug, egg, build_egg): tmpdir = None - if build_egg: - egg, tmpdir = _build_egg() - log("Writing egg to %s" % build_egg) - shutil.copyfile(egg, build_egg) - else: - target = _get_target(target) - project = _get_project(target, project) - version = _get_version(target, version) - if egg: - log("Using egg: %s" % egg) - egg = egg - else: - log("Packing version %s" % version) + try: + if build_egg: egg, tmpdir = _build_egg() - - _upload_egg(target, egg, project, version) - click.echo("Run your spiders at: https://dash.scrapinghub.com/p/%s/" % project) - - if tmpdir: - if debug: - log("Output dir not removed: %s" % tmpdir) + log("Writing egg to %s" % build_egg) + shutil.copyfile(egg, build_egg) else: - shutil.rmtree(tmpdir) + target = _get_target(target) + project = _get_project(target, project) + version = _get_version(target, version) + if egg: + log("Using egg: %s" % egg) + egg = egg + else: + log("Packing version %s" % version) + egg, tmpdir = _build_egg() + + _upload_egg(target, egg, project, version) + click.echo("Run your spiders at: https://dash.scrapinghub.com/p/%s/" % project) + finally: + if tmpdir: + if debug: + log("Output dir not removed: %s" % tmpdir) + else: + shutil.rmtree(tmpdir) def _get_project(target, project):