diff --git a/gradient/cli/common.py b/gradient/cli/common.py index c9136145..e19d32eb 100644 --- a/gradient/cli/common.py +++ b/gradient/cli/common.py @@ -34,12 +34,11 @@ class ClickGroup(DYMMixin, HelpColorsGroup): pass -def deprecated(version="1.0.0"): - deprecated_invoke_notice = """DeprecatedWarning: \nWARNING: This command will not be included in version %s . -For more information, please see: +def deprecated(msg): + deprecated_invoke_notice = msg + """\nFor more information, please see: https://docs.paperspace.com -If you depend on functionality not listed there, please file an issue.""" % version +If you depend on functionality not listed there, please file an issue.""" def new_invoke(self, ctx): click.echo(click.style(deprecated_invoke_notice, fg='red'), err=True) diff --git a/gradient/cli/experiments.py b/gradient/cli/experiments.py index 0f09a320..f218e012 100644 --- a/gradient/cli/experiments.py +++ b/gradient/cli/experiments.py @@ -3,10 +3,10 @@ import click -from gradient import client, config, constants +from gradient import client, config, constants, utils from gradient.cli.cli import cli from gradient.cli.cli_types import json_string, ChoiceType -from gradient.cli.common import api_key_option, del_if_value_is_none, ClickGroup +from gradient.cli.common import api_key_option, del_if_value_is_none, ClickGroup, deprecated from gradient.commands import experiments as experiments_commands MULTI_NODE_EXPERIMENT_TYPES_MAP = collections.OrderedDict( @@ -235,20 +235,26 @@ def common_experiments_create_single_node_options(f): return functools.reduce(lambda x, opt: opt(x), reversed(options), f) +@deprecated("DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @create_experiment.command(name="multinode", help="Create multi node experiment") @common_experiments_create_options @common_experiment_create_multi_node_options def create_multi_node(api_key, **kwargs): + utils.validate_workspace_input(kwargs) del_if_value_is_none(kwargs) experiments_api = client.API(config.CONFIG_EXPERIMENTS_HOST, api_key=api_key) command = experiments_commands.CreateExperimentCommand(api=experiments_api) command.execute(kwargs) +@deprecated("DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @create_experiment.command(name="singlenode", help="Create single node experiment") @common_experiments_create_options @common_experiments_create_single_node_options def create_single_node(api_key, **kwargs): + utils.validate_workspace_input(kwargs) kwargs["experimentTypeId"] = constants.ExperimentType.SINGLE_NODE del_if_value_is_none(kwargs) experiments_api = client.API(config.CONFIG_EXPERIMENTS_HOST, api_key=api_key) @@ -256,6 +262,8 @@ def create_single_node(api_key, **kwargs): command.execute(kwargs) +@deprecated("DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @create_and_start_experiment.command(name="multinode", help="Create and start new multi node experiment") @common_experiments_create_options @common_experiment_create_multi_node_options @@ -269,6 +277,7 @@ def create_single_node(api_key, **kwargs): ) @click.pass_context def create_and_start_multi_node(ctx, api_key, show_logs, **kwargs): + utils.validate_workspace_input(kwargs) del_if_value_is_none(kwargs) experiments_api = client.API(config.CONFIG_EXPERIMENTS_HOST, api_key=api_key) command = experiments_commands.CreateAndStartExperimentCommand(api=experiments_api) @@ -277,6 +286,8 @@ def create_and_start_multi_node(ctx, api_key, show_logs, **kwargs): ctx.invoke(list_logs, experiment_id=experiment["handle"], line=0, limit=100, follow=True, api_key=api_key) +@deprecated("DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @create_and_start_experiment.command(name="singlenode", help="Create and start new single node experiment") @common_experiments_create_options @common_experiments_create_single_node_options @@ -290,6 +301,7 @@ def create_and_start_multi_node(ctx, api_key, show_logs, **kwargs): ) @click.pass_context def create_and_start_single_node(ctx, api_key, show_logs, **kwargs): + utils.validate_workspace_input(kwargs) kwargs["experimentTypeId"] = constants.ExperimentType.SINGLE_NODE del_if_value_is_none(kwargs) experiments_api = client.API(config.CONFIG_EXPERIMENTS_HOST, api_key=api_key) diff --git a/gradient/cli/jobs.py b/gradient/cli/jobs.py index ad00989b..21b69c8b 100644 --- a/gradient/cli/jobs.py +++ b/gradient/cli/jobs.py @@ -2,10 +2,10 @@ import click -from gradient import client, config +from gradient import client, config, utils from gradient.cli.cli import cli from gradient.cli.cli_types import json_string -from gradient.cli.common import api_key_option, del_if_value_is_none, ClickGroup, jsonify_dicts +from gradient.cli.common import api_key_option, del_if_value_is_none, ClickGroup, jsonify_dicts, deprecated from gradient.commands import jobs as jobs_commands @@ -96,11 +96,14 @@ def common_jobs_create_options(f): return functools.reduce(lambda x, opt: opt(x), reversed(options), f) +@deprecated("DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @jobs_group.command("create", help="Create job") @common_jobs_create_options @api_key_option @click.pass_context def create_job(ctx, api_key, **kwargs): + utils.validate_workspace_input(kwargs) del_if_value_is_none(kwargs) jsonify_dicts(kwargs) diff --git a/gradient/cli/run.py b/gradient/cli/run.py index 28b99835..5c8d8862 100644 --- a/gradient/cli/run.py +++ b/gradient/cli/run.py @@ -1,6 +1,6 @@ import click -from gradient import client, config +from gradient import client, config, utils from gradient.cli import common from gradient.cli.cli import cli from gradient.cli.common import del_if_value_is_none, deprecated, jsonify_dicts @@ -9,7 +9,9 @@ from gradient.constants import RunMode -@deprecated(version="0.6.0") +@deprecated("DeprecatedWarning: \nWARNING: This command will not be included in version 0.6.0\n" + "DeprecatedWarning: \nWARNING: --workspaceUrl and --workspaceArchive " + "options will not be included in version 0.6.0") @cli.command("run", help="Run script or command on remote cluster") @click.option("-c", "--python-command", "mode", flag_value=RunMode.RUN_MODE_PYTHON_COMMAND) @click.option("-m", "--module", "mode", flag_value=RunMode.RUN_MODE_PYTHON_MODULE) @@ -18,6 +20,7 @@ @click.argument("script", nargs=-1, required=True) @common.api_key_option def run(api_key, **kwargs): + utils.validate_workspace_input(kwargs) del_if_value_is_none(kwargs) jsonify_dicts(kwargs) diff --git a/gradient/exceptions.py b/gradient/exceptions.py index e7627c2d..a82cb2b1 100644 --- a/gradient/exceptions.py +++ b/gradient/exceptions.py @@ -32,3 +32,11 @@ class PresignedUrlError(ApplicationError): class S3UploadFailedError(ApplicationError): pass + + +class WrongPathError(ApplicationError): + pass + + +class MutuallyExclusiveParametersUsedError(Exception): + pass diff --git a/gradient/utils.py b/gradient/utils.py index d338bd32..a055da05 100644 --- a/gradient/utils.py +++ b/gradient/utils.py @@ -1,9 +1,13 @@ import json +import os import shutil +import click import requests import six +from gradient import exceptions + def get_terminal_lines(fallback=48): if six.PY3: @@ -32,4 +36,58 @@ def status_code_to_error_obj(status_code): message = 'unknown' if status_code in requests.status_codes._codes: message = requests.status_codes._codes[status_code][0] - return { 'error': True, 'message': message, 'status': status_code } \ No newline at end of file + return { 'error': True, 'message': message, 'status': status_code } + + +class PathParser(object): + LOCAL_DIR = 0 + LOCAL_FILE = 1 + GIT_URL = 2 + S3_URL = 3 + + @classmethod + def parse_path(cls, path): + if cls.is_local_dir(path): + return cls.LOCAL_DIR + + if cls.is_local_zip_file(path): + return cls.LOCAL_FILE + + if cls.is_git_url(path): + return cls.GIT_URL + + if cls.is_s3_url(path): + return cls.S3_URL + + raise exceptions.WrongPathError("Given path is neither local path, nor valid URL") + + @staticmethod + def is_local_dir(path): + return os.path.exists(path) and os.path.isdir(path) + + @staticmethod + def is_local_zip_file(path): + return os.path.exists(path) and os.path.isfile(path) and path.endswith(".zip") + + @staticmethod + def is_git_url(path): + return not os.path.exists(path) and path.endswith(".git") or path.lower().startswith("git:") + + @staticmethod + def is_s3_url(path): + return not os.path.exists(path) and path.lower().startswith("s3:") + + +def validate_workspace_input(input_data): + workspace_url = input_data.get('workspaceUrl') + workspace_path = input_data.get('workspace') + workspace_archive = input_data.get('workspaceArchive') + + if (workspace_archive and workspace_path) \ + or (workspace_archive and workspace_url) \ + or (workspace_path and workspace_url): + raise click.UsageError("Use either:\n\t--workspace https://path.to/git/repository.git - to point repository URL" + "\n\t--workspace /path/to/local/directory - to point on project directory" + "\n\t--workspace /path/to/local/archive.zip - to point on project .zip archive" + "\n\t--workspace none - to use no workspace" + "\n or neither to use current directory") diff --git a/gradient/workspace.py b/gradient/workspace.py index 47fd90f8..7d2b2856 100644 --- a/gradient/workspace.py +++ b/gradient/workspace.py @@ -7,7 +7,7 @@ import requests from requests_toolbelt.multipart import encoder -from gradient import logger +from gradient import logger, utils from gradient.exceptions import S3UploadFailedError, PresignedUrlUnreachableError, \ PresignedUrlAccessDeniedError, PresignedUrlConnectionError, ProjectAccessDeniedError, \ PresignedUrlMalformedResponseError, PresignedUrlError @@ -120,16 +120,26 @@ def handle(self, input_data): @staticmethod def _validate_input(input_data): + utils.validate_workspace_input(input_data) + workspace_url = input_data.get('workspaceUrl') workspace_path = input_data.get('workspace') workspace_archive = input_data.get('workspaceArchive') - if (workspace_archive and workspace_path) or (workspace_archive and workspace_url) or ( - workspace_path and workspace_url): - raise click.UsageError("Use either:\n\t--workspaceUrl to point repository URL" - "\n\t--workspace to point on project directory" - "\n\t--workspaceArchive to point on project .zip archive" - "\n or neither to use current directory") + if workspace_path not in ("none", None): + path_type = utils.PathParser().parse_path(workspace_path) + + if path_type == utils.PathParser.LOCAL_DIR: + input_data["workspace"] = workspace_path + else: + if path_type == utils.PathParser.LOCAL_FILE: + input_data["workspaceArchive"] = workspace_archive = workspace_path + elif path_type in (utils.PathParser.GIT_URL, utils.PathParser.S3_URL): + input_data["workspaceUrl"] = workspace_url = workspace_path + + workspace_path = None + input_data.pop("workspace", None) + return workspace_archive, workspace_path, workspace_url diff --git a/tests/functional/test_experiments.py b/tests/functional/test_experiments.py index da5c8c02..bdd687f4 100644 --- a/tests/functional/test_experiments.py +++ b/tests/functional/test_experiments.py @@ -87,7 +87,7 @@ def test_should_send_proper_data_and_print_message_when_create_experiment_was_ru params=None, files=None, data=None) - assert result.output == self.EXPECTED_STDOUT + assert self.EXPECTED_STDOUT in result.output assert result.exit_code == 0 @mock.patch("gradient.client.requests.post") @@ -104,7 +104,7 @@ def test_should_send_proper_data_and_print_message_when_create_experiment_was_ru params=None, files=None, data=None) - assert result.output == self.EXPECTED_STDOUT + assert self.EXPECTED_STDOUT in result.output assert result.exit_code == 0 assert self.EXPECTED_HEADERS_WITH_CHANGED_API_KEY["X-API-Key"] == "some_key" @@ -122,7 +122,7 @@ def test_should_send_proper_data_and_print_message_when_create_wrong_project_id_ params=None, files=None, data=None) - assert result.output == self.EXPECTED_STDOUT_PROJECT_NOT_FOUND + assert self.EXPECTED_STDOUT_PROJECT_NOT_FOUND in result.output assert result.exit_code == 0 @@ -145,7 +145,7 @@ class TestExperimentsCreateMultiNode(object): "--parameterServerCommand", "ls", "--parameterServerCount", 2, "--workerContainerUser", "usr", - "--workspaceUrl", "some-workspace", + "--workspace", "https://github.com/Paperspace/gradient-cli.git", ] FULL_OPTIONS_COMMAND = [ "experiments", "create", "multinode", @@ -187,7 +187,7 @@ class TestExperimentsCreateMultiNode(object): "parameterServerCommand": u"ls", "parameterServerCount": 2, "workerContainerUser": u"usr", - "workspaceUrl": "some-workspace", + "workspaceUrl": "https://github.com/Paperspace/gradient-cli.git", } FULL_OPTIONS_REQUEST = { "name": u"multinode_mpi", @@ -232,7 +232,7 @@ def test_should_send_proper_data_and_print_message_when_create_experiment_was_ru params=None, files=None, data=None) - assert result.output == self.EXPECTED_STDOUT + assert self.EXPECTED_STDOUT in result.output assert result.exit_code == 0 @mock.patch("gradient.client.requests.post") @@ -249,7 +249,7 @@ def test_should_send_proper_data_and_print_message_when_create_experiment_was_ru params=None, files=None, data=None) - assert result.output == self.EXPECTED_STDOUT + assert self.EXPECTED_STDOUT in result.output assert result.exit_code == 0 @@ -304,7 +304,7 @@ class TestExperimentsCreateAndStartMultiNode(TestExperimentsCreateMultiNode): "--parameterServerCommand", "ls", "--parameterServerCount", 2, "--workerContainerUser", "usr", - "--workspaceUrl", "some-workspace", + "--workspace", "https://github.com/Paperspace/gradient-cli.git", "--no-logs", ] FULL_OPTIONS_COMMAND = [ diff --git a/tests/functional/test_jobs.py b/tests/functional/test_jobs.py index 318e6e5b..2c7aace9 100644 --- a/tests/functional/test_jobs.py +++ b/tests/functional/test_jobs.py @@ -357,7 +357,7 @@ class TestJobsCreate(object): "--container", "testContainer", "--machineType", "testType", "--command", "testCommand", - "--workspaceUrl", "some-workspace", + "--workspace", "https://github.com/Paperspace/gradient-cli.git", ] FULL_OPTIONS_COMMAND = [ "jobs", "create", @@ -383,8 +383,8 @@ class TestJobsCreate(object): "container": u"testContainer", "machineType": u"testType", "command": u"testCommand", - "workspaceUrl": u"some-workspace", - "workspaceFileName": u"some-workspace", + "workspaceUrl": u"https://github.com/Paperspace/gradient-cli.git", + "workspaceFileName": u"https://github.com/Paperspace/gradient-cli.git", } FULL_OPTIONS_REQUEST = { "name": u"exp1", @@ -424,5 +424,5 @@ def test_should_send_proper_data_and_print_message_when_create_job_was_run_with_ files=None, data=None) - assert result.output == self.EXPECTED_STDOUT + assert self.EXPECTED_STDOUT in result.output assert result.exit_code == 0 diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py new file mode 100644 index 00000000..e08d60ab --- /dev/null +++ b/tests/unit/test_utils.py @@ -0,0 +1,75 @@ +import mock +import pytest + +from gradient import exceptions +from gradient.utils import PathParser + + +class TestPathParser(object): + @mock.patch("os.path.exists", return_value=True) + @mock.patch("os.path.isdir", return_value=True) + def test_should_return_local_dir_type_when_given_directory_exists(self, _, __): + path_str = "/home/usr/some/path" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.LOCAL_DIR + + @mock.patch("os.path.exists", return_value=True) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=True) + def test_should_return_local_archive_type_when_archive_exists(self, _, __, ___): + path_str = "/home/usr/some/path.zip" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.LOCAL_FILE + + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=False) + def test_should_return_git_url_type_when_git_ssh_url_was_given(self, _, __, ___): + path_str = "git@github.com:Paperspace/gradient-cli.git" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.GIT_URL + + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=False) + def test_should_return_git_url_type_when_git_http_url_was_given(self, _, __, ___): + path_str = "https://github.com/Paperspace/gradient-cli.git" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.GIT_URL + + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=False) + def test_should_return_git_url_type_when_git_http_url_was_given_with_git_prefix_and_no_extension(self, _, __, ___): + path_str = "GIT:https://github.com/Paperspace/gradient-cli" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.GIT_URL + + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=False) + def test_should_return_s3_type_when_s3_url_was_given(self, _, __, ___): + path_str = "S3:some/path" + + path_type = PathParser.parse_path(path_str) + + assert path_type == PathParser.S3_URL + + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.path.isdir", return_value=False) + @mock.patch("os.path.isfile", return_value=False) + def test_should_raise_exception_when_local_path_was_given_but_does_not_exist(self, _, __, ___): + path_str = "/home/usr/some/path.zip" + + with pytest.raises(exceptions.WrongPathError): + PathParser.parse_path(path_str) diff --git a/tests/unit/test_workspace.py b/tests/unit/test_workspace.py index 4183dc25..34a9b472 100644 --- a/tests/unit/test_workspace.py +++ b/tests/unit/test_workspace.py @@ -4,7 +4,7 @@ import mock import pytest -from gradient import exceptions +from gradient import exceptions, utils from gradient.workspace import S3WorkspaceHandler MOCK_BUCKET_NAME = 'bucket_name' @@ -43,12 +43,14 @@ def test_raise_exception_when_more_than_one_workspace_provided(self, params, wor with pytest.raises(click.UsageError): workspace_handler.handle(params) - def test_dont_upload_if_archive_url_provided(self, workspace_handler): + @mock.patch("gradient.utils.PathParser.parse_path", return_value=utils.PathParser.LOCAL_FILE) + def test_dont_upload_if_archive_path_provided(self, _, workspace_handler): workspace_handler.handle({'workspaceUrl': 'foo'}) workspace_handler._upload.assert_not_called() - def test_zip_files_and_receive_s3_response_when_no_dir_provided(self, workspace_handler): + @mock.patch("gradient.utils.PathParser.parse_path", return_value=None) + def test_zip_files_and_receive_s3_response_when_no_dir_provided(self, _, workspace_handler): archive_name = 'foo.zip' workspace_handler._zip_workspace = mock.MagicMock() @@ -61,7 +63,8 @@ def test_zip_files_and_receive_s3_response_when_no_dir_provided(self, workspace_ workspace_handler._upload.assert_called_with(archive_name, mock_upload_data) assert response_url == 's3://{}/{}'.format(MOCK_BUCKET_NAME, MOCK_OBJECT_KEY) - def test_zip_files_and_receive_s3_response_when_workspace_dir_provided(self, workspace_handler): + @mock.patch("gradient.utils.PathParser.parse_path", return_value=utils.PathParser.LOCAL_DIR) + def test_zip_files_and_receive_s3_response_when_workspace_dir_provided(self, _, workspace_handler): archive_name = 'foo.zip' workspace_handler._zip_workspace = mock.MagicMock() @@ -74,7 +77,8 @@ def test_zip_files_and_receive_s3_response_when_workspace_dir_provided(self, wor workspace_handler._upload.assert_called_with(archive_name, mock_upload_data) assert response_url == 's3://{}/{}'.format(MOCK_BUCKET_NAME, MOCK_OBJECT_KEY) - def test_dont_zip_files_and_receive_s3_response_when_workspace_archive_provided(self, workspace_handler): + @mock.patch("gradient.utils.PathParser.parse_path", return_value=utils.PathParser.LOCAL_FILE) + def test_dont_zip_files_and_receive_s3_response_when_workspace_archive_provided(self, _, workspace_handler): workspace_handler._zip_workspace = mock.MagicMock() response_url = workspace_handler.handle({'projectHandle': 'foo', 'workspaceArchive': 'foo.zip'}) @@ -84,6 +88,28 @@ def test_dont_zip_files_and_receive_s3_response_when_workspace_archive_provided( workspace_handler._upload.assert_called_with(os.path.abspath('foo.zip'), mock_upload_data) assert response_url == 's3://{}/{}'.format(MOCK_BUCKET_NAME, MOCK_OBJECT_KEY) + @mock.patch("gradient.utils.PathParser.parse_path", return_value=utils.PathParser.LOCAL_FILE) + def test_dont_zip_files_and_receive_s3_response_when_workspace_archive_provided_with_workspace(self, _, workspace_handler): + workspace_handler._zip_workspace = mock.MagicMock() + + response_url = workspace_handler.handle({'projectHandle': 'foo', 'workspace': 'foo.zip'}) + + workspace_handler._zip_workspace.assert_not_called() + workspace_handler._upload.assert_called_once() + workspace_handler._upload.assert_called_with(os.path.abspath('foo.zip'), mock_upload_data) + assert response_url == 's3://{}/{}'.format(MOCK_BUCKET_NAME, MOCK_OBJECT_KEY) + + @mock.patch("gradient.utils.PathParser.parse_path", return_value=utils.PathParser.LOCAL_FILE) + def test_dont_zip_files_and_receive_s3_response_when_workspace_archive_provided_with_workspace(self, _, workspace_handler): + workspace_handler._zip_workspace = mock.MagicMock() + + response_url = workspace_handler.handle({'projectHandle': 'foo', 'workspace': 'foo.zip'}) + + workspace_handler._zip_workspace.assert_not_called() + workspace_handler._upload.assert_called_once() + workspace_handler._upload.assert_called_with(os.path.abspath('foo.zip'), mock_upload_data) + assert response_url == 's3://{}/{}'.format(MOCK_BUCKET_NAME, MOCK_OBJECT_KEY) + @pytest.mark.parametrize('code,exception', ((401, exceptions.ProjectAccessDeniedError), (403, exceptions.PresignedUrlAccessDeniedError), (404, exceptions.PresignedUrlUnreachableError)))