diff --git a/onecodex/cli.py b/onecodex/cli.py index 6f68ea24..103f239c 100644 --- a/onecodex/cli.py +++ b/onecodex/cli.py @@ -4,7 +4,6 @@ author: @mbiokyle29 """ from __future__ import print_function -from functools import wraps import logging import os import platform @@ -17,7 +16,7 @@ from onecodex.utils import (cli_resource_fetcher, download_file_helper, valid_api_key, OPTION_HELP, pprint, - warn_if_insecure_platform) + warn_if_insecure_platform, telemetry) from onecodex.api import Api from onecodex.exceptions import ValidationWarning, ValidationError, UploadException from onecodex.auth import _login, _logout, _silent_login @@ -50,9 +49,19 @@ help=OPTION_HELP['no_telemetry']) @click.version_option(version=__version__) @click.pass_context +@telemetry def onecodex(ctx, api_key, no_pprint, verbose, no_telemetry): """One Codex v1 API command line interface""" - print('onecodex') + # set up Sentry + if not (no_telemetry or os.environ.get('ONE_CODEX_NO_TELEMETRY') is not None): + key = 'f74d421bbdb44ea78de504f25067b9a4:16c934ef082e40efaf51958d8521c32d' + try: + client = Client(dsn="https://{}@sentry.onecodex.com/9".format(key), release=__version__) + client.extra_context({'platform': platform.platform()}) + ctx.obj['sentry'] = client + except Exception: + pass + # set up the context for sub commands click.Context.get_usage = click.Context.get_help ctx.obj = {} @@ -81,32 +90,11 @@ def onecodex(ctx, api_key, no_pprint, verbose, no_telemetry): if ctx.invoked_subcommand != "upload": warn_if_insecure_platform() - if not (no_telemetry or os.environ.get('ONE_CODEX_NO_TELEMETRY') is not None): - key = 'f74d421bbdb44ea78de504f25067b9a4:16c934ef082e40efaf51958d8521c32d' - client = Client(dsn="https://{}@sentry.onecodex.com/9".format(key), release=__version__) - client.extra_context({'platform': platform.platform()}) - ctx.obj['sentry'] = client - - -def telemetry(fn): - @wraps(fn) - def telemetry_wrapper(*args, **kwargs): - try: - return fn(*args, **kwargs) - except SystemExit as e: - sys.exit(e.code) # make sure we still exit with the proper code - except: - ctx = args[0] - if 'sentry' in ctx.obj: - ctx.obj['sentry'].captureException() - return telemetry_wrapper - # resources @onecodex.command('analyses') @click.argument('analyses', nargs=-1, required=False) @click.pass_context -@telemetry def analyses(ctx, analyses): """Retrieve performed analyses""" cli_resource_fetcher(ctx, "analyses", analyses) @@ -121,7 +109,6 @@ def analyses(ctx, analyses): help=OPTION_HELP['results']) @click.pass_context @click.argument('classifications', nargs=-1, required=False) -@telemetry def classifications(ctx, classifications, results, readlevel, readlevel_path): """Retrieve performed metagenomic classifications""" @@ -157,7 +144,6 @@ def classifications(ctx, classifications, results, readlevel, readlevel_path): @onecodex.command('panels') @click.pass_context @click.argument('panels', nargs=-1, required=False) -@telemetry def panels(ctx, panels): """Retrieve performed in silico panel results""" cli_resource_fetcher(ctx, "panels", panels) @@ -166,7 +152,6 @@ def panels(ctx, panels): @onecodex.command('samples') @click.pass_context @click.argument('samples', nargs=-1, required=False) -@telemetry def samples(ctx, samples): """Retrieve uploaded samples""" cli_resource_fetcher(ctx, "samples", samples) @@ -252,7 +237,6 @@ def upload(ctx, files, max_threads, clean, no_interleave, prompt, validate): @onecodex.command('login') @click.pass_context -@telemetry def login(ctx): """Add an API key (saved in ~/.onecodex)""" base_url = os.environ.get("ONE_CODEX_API_BASE", "https://app.onecodex.com") @@ -261,7 +245,6 @@ def login(ctx): @onecodex.command('logout') @click.pass_context -@telemetry def logout(ctx): """Delete your API key (saved in ~/.onecodex)""" _logout() diff --git a/onecodex/utils.py b/onecodex/utils.py index 1d941e13..3e31d12c 100644 --- a/onecodex/utils.py +++ b/onecodex/utils.py @@ -2,6 +2,7 @@ utils.py author: @mbiokyle29 """ +from functools import wraps import importlib import json import logging @@ -194,6 +195,24 @@ def collapse_user(fp): return abs_path.replace(home_dir, "~") +def telemetry(fn): + @wraps(fn) + def telemetry_wrapper(*args, **kwargs): + try: + return fn(*args, **kwargs) + except SystemExit as e: + ctx = args[0] + if 'sentry' in ctx.obj: + ctx.obj['sentry'].captureException() + sys.exit(e.code) # make sure we still exit with the proper code + except Exception as e: + ctx = args[0] + if 'sentry' in ctx.obj: + ctx.obj['sentry'].captureException() + raise e + return telemetry_wrapper + + class ModuleAlias(object): # Used as a proxy object to attach # all of a module's __all__ namespace to diff --git a/tests/test_cli.py b/tests/test_cli.py index b2ed3cb9..4d83c896 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -40,15 +40,15 @@ def test_cli_wo_override(api_data, monkeypatch): # Analyses def test_analysis_help(runner, api_data, mocked_creds_file): - result = runner.invoke(Cli, ['analyses', '--help']) + result = runner.invoke(Cli, ['--no-telemetry', 'analyses', '--help']) analysis_desc = "Retrieve performed analyses" assert result.exit_code == 0 assert analysis_desc in result.output def test_analyses(runner, api_data, mocked_creds_file): - r0 = runner.invoke(Cli, ['analyses']) - r1 = runner.invoke(Cli, ['analyses', '593601a797914cbf']) + r0 = runner.invoke(Cli, ['--no-telemetry', 'analyses']) + r1 = runner.invoke(Cli, ['--no-telemetry', 'analyses', '593601a797914cbf']) assert r0.exit_code == 0 assert r1.exit_code == 0 assert API_DATA['GET::api/v1/analyses/593601a797914cbf']['$uri'] in r0.output @@ -57,27 +57,27 @@ def test_analyses(runner, api_data, mocked_creds_file): # Classifications def test_classification_instance(runner, api_data, mocked_creds_file): - result = runner.invoke(Cli, ['classifications', '593601a797914cbf']) + result = runner.invoke(Cli, ['--no-telemetry', 'classifications', '593601a797914cbf']) assert result.exit_code == 0 assert API_DATA['GET::api/v1/classifications/593601a797914cbf']['$uri'] in result.output def test_classifications_table(runner, api_data, mocked_creds_file, monkeypatch): - result = runner.invoke(Cli, ['classifications', '45a573fb7833449a', '--results']) + result = runner.invoke(Cli, ['--no-telemetry', 'classifications', '45a573fb7833449a', '--results']) assert result.exit_code == 0 assert "Staphylococcus" in result.output # Panels def test_panel_instances(runner, api_data, mocked_creds_file): - result = runner.invoke(Cli, ['panels']) + result = runner.invoke(Cli, ['--no-telemetry', 'panels']) assert result.exit_code == 0 # Samples def test_samples(runner, api_data, mocked_creds_file): - r0 = runner.invoke(Cli, ['samples']) - r1 = runner.invoke(Cli, ['samples', '7428cca4a3a04a8e']) + r0 = runner.invoke(Cli, ['--no-telemetry', 'samples']) + r1 = runner.invoke(Cli, ['--no-telemetry', 'samples', '7428cca4a3a04a8e']) assert r0.exit_code == 0 assert r1.exit_code == 0 assert API_DATA['GET::api/v1/samples/7428cca4a3a04a8e']['$uri'] in r0.output @@ -102,7 +102,7 @@ def test_api_login(runner, mocked_creds_path): login_input = 'user@example.com' + '\n' + 'userpassword' + '\n' successful_login_msg = 'Your ~/.onecodex credentials file was successfully created.' with Replace('onecodex.auth.fetch_api_key_from_uname', mock_fetch_api_key): - result = runner.invoke(Cli, ['login'], input=login_input) + result = runner.invoke(Cli, ['--no-telemetry', 'login'], input=login_input) assert result.exit_code == 0 assert successful_login_msg in result.output @@ -112,7 +112,7 @@ def test_creds_file_exists(runner, mocked_creds_file): make_creds_file() expected_message = "Credentials file already exists" - result = runner.invoke(Cli, ["login"]) + result = runner.invoke(Cli, ['--no-telemetry', "login"]) assert result.exit_code == 0 assert expected_message in result.output @@ -120,7 +120,7 @@ def test_creds_file_exists(runner, mocked_creds_file): def test_silent_login(runner, mocked_creds_file, api_data): with runner.isolated_filesystem(): make_creds_file() - result = runner.invoke(Cli, ['samples']) + result = runner.invoke(Cli, ['--no-telemetry', 'samples']) assert result.exit_code == 0 @@ -140,7 +140,7 @@ def test_logout_creds_exists(runner, mocked_creds_file): make_creds_file() expected_message = "Successfully removed One Codex credentials." path = os.path.expanduser("~/.onecodex") - result = runner.invoke(Cli, ["logout"]) + result = runner.invoke(Cli, ['--no-telemetry', 'logout']) assert result.exit_code == 0 assert expected_message in result.output assert os.path.exists(path) is False @@ -148,7 +148,7 @@ def test_logout_creds_exists(runner, mocked_creds_file): def test_logout_creds_dne(runner, mocked_creds_path): expected_message = "No One Codex API keys found." - result = runner.invoke(Cli, ["logout"]) + result = runner.invoke(Cli, ['--no-telemetry', 'logout']) assert result.exit_code == 1 assert expected_message in result.output @@ -169,7 +169,7 @@ def test_standard_uploads(runner, upload_mocks, files, threads): (but not files >5GB) """ with runner.isolated_filesystem(): - args = ['--api-key', '01234567890123456789012345678901', 'upload'] + args = ['--no-telemetry', '--api-key', '01234567890123456789012345678901', 'upload'] if not threads: args += ['--max-threads', '1'] for f in files: @@ -188,7 +188,7 @@ def test_empty_upload(runner, upload_mocks): f = 'tmp.fa' f_out = open(f, mode='w') f_out.close() - args = ['--api-key', '01234567890123456789012345678901', 'upload', f] + args = ['--no-telemetry', '--api-key', '01234567890123456789012345678901', 'upload', f] result = runner.invoke(Cli, args) assert result.exit_code != 0 @@ -204,7 +204,7 @@ def test_paired_files(runner, upload_mocks): f_out2.write('>Test fasta\n') f_out2.write(SEQUENCE) - args = ['--api-key', '01234567890123456789012345678901', 'upload', f, f2] + args = ['--no-telemetry', '--api-key', '01234567890123456789012345678901', 'upload', f, f2] # check that only one upload is kicked off for the pair of files patch1 = 'onecodex.lib.upload.upload_file' patch2 = 'onecodex.lib.inline_validator.FASTXTranslator.close' @@ -216,13 +216,13 @@ def test_paired_files(runner, upload_mocks): assert result.exit_code == 0 # Check with validate=False, should fail - args = ['--api-key', '01234567890123456789012345678901', 'upload', f, f2, + args = ['--no-telemetry', '--api-key', '01234567890123456789012345678901', 'upload', f, f2, '--do-not-validate'] result2 = runner.invoke(Cli, args) assert result2.exit_code != 0 # Check with validate=False, interleave=False, should success - args = ['--api-key', '01234567890123456789012345678901', 'upload', f, f2, + args = ['--no-telemetry', '--api-key', '01234567890123456789012345678901', 'upload', f, f2, '--do-not-validate', '--do-not-interleave'] with mock.patch(patch1) as mp: result3 = runner.invoke(Cli, args) @@ -248,7 +248,8 @@ def mockfilesize(path): f.write('>BIG!!!\n') f.write(SEQUENCE) - args = ['--api-key', '01234567890123456789012345678901', 'upload', big_file] + args = ['--no-telemetry', '--api-key', + '01234567890123456789012345678901', 'upload', big_file] def side_effect(*args, **kwargs): """Side effect to ensure FASTXValidator gets properly read