diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 0e853f18..bbb44bca 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -25,10 +25,6 @@ jobs: flake8 substra - name: Install substra run: pip install -e . - - name: Generate and validate CLI documentation - run: | - pip install pydantic==1.8.2 - python bin/generate_cli_documentation.py --output-path references/cli.md - name: Generate and validate SDK documentation run: | python bin/generate_sdk_documentation.py --output-path='references/sdk.md' diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a078600..a13a2935 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - Function name from the Performance model ([#358](https://github.com/Substra/substra/pull/358)) +- BREAKING: `substra.cli` module ([#362](https://github.com/Substra/substra/pull/362)) +- BREAKING: `Client.from_config_file()`. The recommended way is to use the new config file format or env vars ([#362](https://github.com/Substra/substra/pull/362)) ## [0.43.0](https://github.com/Substra/substra/releases/tag/0.43.0) - 2023-03-31 diff --git a/Makefile b/Makefile index 939c8eb9..ef7d87fe 100644 --- a/Makefile +++ b/Makefile @@ -5,15 +5,12 @@ pyclean: find . -type d -name "__pycache__" -delete rm -rf build/ dist/ *.egg-info -doc-cli: pyclean - python bin/generate_cli_documentation.py - doc-sdk: pyclean python bin/generate_sdk_documentation.py python bin/generate_sdk_schemas_documentation.py python bin/generate_sdk_schemas_documentation.py --models --output-path='references/sdk_models.md' -doc: doc-cli doc-sdk +doc: doc-sdk test: pyclean pytest tests diff --git a/bin/generate_cli_documentation.py b/bin/generate_cli_documentation.py deleted file mode 100644 index 9151d94d..00000000 --- a/bin/generate_cli_documentation.py +++ /dev/null @@ -1,83 +0,0 @@ -import argparse -import os -import subprocess -import sys - -import click - -from substra.cli.interface import cli - -local_dir = os.path.dirname(os.path.abspath(__file__)) - - -def _click_parse_node(name, command, parent, callback): - ctx = click.Context(command, info_name=name, parent=parent) - if not hasattr(ctx.command, "commands"): - callback(ctx.command_path) - return - - # command definitions are sorted in the python script as required for the - # documentation - for _, c in ctx.command.commands.items(): - _click_parse_node(c.name, c, ctx, callback) - - -def click_get_commands(name, command): - commands = [] - - def cb(command_path): - commands.append(command_path) - - _click_parse_node(name, command, None, cb) - return commands - - -def generate_help(commands, fh): - # TODO use click context to generate help page: - # https://github.com/click-contrib/click-man/blob/master/click_man/core.py#L20 - fh.write("# Summary\n\n") - - def _create_anchor(command): - return "#{}".format(command.replace(" ", "-")) - - # XXX order when iterating on commands items must be consistent - for command in commands: - anchor = _create_anchor(command) - fh.write(f"- [{command}]({anchor})\n") - - fh.write("\n\n") - fh.write("# Commands\n\n") - - for command in commands: - anchor = _create_anchor(command) - command_args = command.split(" ") - command_args.append("--help") - command_helper = subprocess.check_output(command_args) - command_helper = command_helper.decode("utf-8") - - fh.write(f"## {command}\n\n") - fh.write("```text\n") - fh.write(command_helper) - fh.write("```\n\n") - - -def write_help(path): - commands = click_get_commands("substra", cli) - with open(path, "w") as fh: - generate_help(commands, fh) - - -if __name__ == "__main__": - - def _cb(args): - write_help(args.output_path) - - doc_dir = os.path.join(local_dir, "../references") - default_path = os.path.join(doc_dir, "cli.md") - - parser = argparse.ArgumentParser() - parser.add_argument("--output-path", type=str, default=default_path, required=False) - parser.set_defaults(func=_cb) - - args = parser.parse_args(sys.argv[1:]) - args.func(args) diff --git a/references/cli.md b/references/cli.md deleted file mode 100644 index 3210fb1e..00000000 --- a/references/cli.md +++ /dev/null @@ -1,86 +0,0 @@ -# Summary - -- [substra config](#substra-config) -- [substra login](#substra-login) -- [substra organization info](#substra-organization-info) -- [substra cancel compute_plan](#substra-cancel-compute_plan) - - -# Commands - -## substra config - -```text -Usage: substra config [OPTIONS] URL - - Add profile to config file. - -Options: - --config PATH Config path (default ~/.substra). - --profile TEXT Profile name to add - -k, --insecure Do not verify SSL certificates - --help Show this message and exit. -``` - -## substra login - -```text -Usage: substra login [OPTIONS] - - Login to the Substra platform. - -Options: - --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL] - Enable logging and set log level - --config PATH Config path (default ~/.substra). - --profile TEXT Profile name to use. - --tokens FILE Tokens file path to use (default ~/.substra- - tokens). - --verbose Enable verbose mode. - -u, --username TEXT - -p, --password TEXT - --help Show this message and exit. -``` - -## substra organization info - -```text -Usage: substra organization info [OPTIONS] - - Display organization info. - -Options: - --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL] - Enable logging and set log level - --config PATH Config path (default ~/.substra). - --profile TEXT Profile name to use. - --tokens FILE Tokens file path to use (default ~/.substra- - tokens). - --verbose Enable verbose mode. - -o, --output [pretty|yaml|json] - Set output format - - pretty: summarised view - - yaml: full view in YAML format - - json: full view in JSON format - [default: pretty] - --help Show this message and exit. -``` - -## substra cancel compute_plan - -```text -Usage: substra cancel compute_plan [OPTIONS] COMPUTE_PLAN_KEY - - Cancel execution of a compute plan. - -Options: - --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL] - Enable logging and set log level - --config PATH Config path (default ~/.substra). - --profile TEXT Profile name to use. - --tokens FILE Tokens file path to use (default ~/.substra- - tokens). - --verbose Enable verbose mode. - --help Show this message and exit. -``` - diff --git a/references/sdk.md b/references/sdk.md index 099a8e3f..3e766005 100644 --- a/references/sdk.md +++ b/references/sdk.md @@ -287,37 +287,6 @@ class. **Returns:** - `pathlib.Path`: Path of the downloaded model -## from_config_file -```text -from_config_file(profile_name: str = 'default', config_path: Union[str, pathlib.Path] = '~/.substra', tokens_path: Union[str, pathlib.Path] = '~/.substra-tokens', token: Optional[str] = None, retry_timeout: int = 300, backend_type: substra.sdk.schemas.BackendType = ) -``` - -Returns a new Client configured with profile data from configuration files. - -**Arguments:** - - `profile_name (str, optional)`: Name of the profile to load. -Defaults to 'default'. - - `config_path (Union[str, pathlib.Path], optional)`: Path to the -configuration file. -Defaults to '~/.substra'. - - `tokens_path (Union[str, pathlib.Path], optional)`: Path to the tokens file. -Defaults to '~/.substra-tokens'. - - `token (str, optional)`: Token to use for authentication (will be used -instead of any token found at tokens_path). Defaults to None. - - `retry_timeout (int, optional)`: Number of seconds before attempting a retry call in case -of timeout. Defaults to 5 minutes. - - `backend_type (schemas.BackendType, optional)`: Which mode to use. -Possible values are `remote`, `docker` and `subprocess`. -Defaults to `remote`. -In `remote` mode, assets are registered on a deployed platform which also executes the tasks. -In `subprocess` or `docker` mode, if no URL is given then all assets are created locally and tasks are -executed locally. If a URL is given then the mode is a hybrid one: new assets are -created locally but can access assets from the deployed Substra platform. The platform is in read-only -mode and tasks are executed locally. - -**Returns:** - - - `Client`: The new client. ## get_compute_plan ```text get_compute_plan(self, key: str) -> substra.sdk.models.ComputePlan diff --git a/setup.py b/setup.py index 3f55e631..bd9d0fbb 100644 --- a/setup.py +++ b/setup.py @@ -39,12 +39,10 @@ packages=find_packages(exclude=["tests*"]), include_package_data=True, install_requires=[ - "click>=7.1.1,!=8.0.0", # issue with click 8.0.0 (#4) "requests", "docker", "pyyaml", "pydantic>=1.5.1", - "six", "tqdm", "python-slugify", ], @@ -62,10 +60,5 @@ "docstring_parser", ], }, - entry_points={ - "console_scripts": [ - "substra=substra.cli.interface:cli", - ], - }, zip_safe=False, ) diff --git a/substra/cli/__init__.py b/substra/cli/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/substra/cli/interface.py b/substra/cli/interface.py deleted file mode 100644 index 11f203c3..00000000 --- a/substra/cli/interface.py +++ /dev/null @@ -1,328 +0,0 @@ -import functools -import json -import logging -import os - -import click - -from substra import __version__ -from substra.cli import printers -from substra.sdk import config as configuration -from substra.sdk import exceptions -from substra.sdk import utils -from substra.sdk.client import Client - -DEFAULT_RETRY_TIMEOUT = 300 - - -def get_client(global_conf): - """Initialize substra client from config file, profile name and tokens file.""" - help_command = "substra config ..." - - try: - client = Client.from_config_file( - config_path=global_conf.config, - profile_name=global_conf.profile, - tokens_path=global_conf.tokens, - ) - - except FileNotFoundError: - raise click.ClickException(f"Config file '{global_conf.config}' not found. Please run '{help_command}'.") - - except configuration.ProfileNotFoundError: - raise click.ClickException(f"Profile '{global_conf.profile}' not found. Please run '{help_command}'.") - - return client - - -def display(res): - """Display result.""" - if res is None: - return - if isinstance(res, dict) or isinstance(res, list): - res = json.dumps(res, indent=2) - print(res) - - -class GlobalConf: - def __init__(self): - self.profile = None - self.config = None - self.tokens = None - self.verbose = None - self.output_format = None - self.retry_timeout = DEFAULT_RETRY_TIMEOUT - - def retry(self, func): - return utils.retry_on_exception( - exceptions=(exceptions.RequestTimeout), - timeout=float(self.retry_timeout), - )(func) - - -def update_global_conf(ctx, param, value): - setattr(ctx.obj, param.name, value) - - -def click_global_conf_profile(f): - """Add profile option to command.""" - return click.option( - "--profile", - expose_value=False, - callback=update_global_conf, - default="default", - help="Profile name to use.", - )(f) - - -def click_global_conf_tokens(f): - """Add tokens option to command.""" - return click.option( - "--tokens", - expose_value=False, - type=click.Path(dir_okay=False, resolve_path=True), - callback=update_global_conf, - default=os.path.expanduser(configuration.DEFAULT_TOKENS_PATH), - help=f"Tokens file path to use (default {configuration.DEFAULT_TOKENS_PATH}).", - )(f) - - -def click_global_conf_config(f): - """Add config option to command.""" - return click.option( - "--config", - expose_value=False, - type=click.Path(exists=True, resolve_path=True), - callback=update_global_conf, - default=os.path.expanduser(configuration.DEFAULT_PATH), - help=f"Config path (default {configuration.DEFAULT_PATH}).", - )(f) - - -def click_global_conf_verbose(f): - """Add verbose option to command.""" - return click.option( - "--verbose", - expose_value=False, - callback=update_global_conf, - is_flag=True, - help="Enable verbose mode.", - )(f) - - -def set_log_level(ctx, param, value): - if value: - logging.basicConfig(level=getattr(logging, value)) - - -def click_global_conf_log_level(f): - """Add verbose option to command.""" - return click.option( - "--log-level", - type=click.Choice(["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]), - callback=set_log_level, - expose_value=False, - help="Enable logging and set log level", - )(f) - - -def click_global_conf_output_format(f): - """Add output option to command.""" - return click.option( - "-o", - "--output", - "output_format", - type=click.Choice(["pretty", "yaml", "json"]), - expose_value=False, - default="pretty", - show_default=True, - callback=update_global_conf, - help="Set output format \ - - pretty: summarised view \ - - yaml: full view in YAML format \ - - json: full view in JSON format \ - ", - )(f) - - -def click_global_conf(f): - f = click_global_conf_verbose(f) - f = click_global_conf_tokens(f) - f = click_global_conf_profile(f) - f = click_global_conf_config(f) - f = click_global_conf_log_level(f) - return f - - -def click_global_conf_with_output_format(f): - f = click_global_conf_output_format(f) - f = click_global_conf(f) - return f - - -def click_option_expand(f): - """Add expand option to command.""" - return click.option("--expand", is_flag=True, help="Display associated assets details")(f) - - -def click_global_conf_retry_timeout(f): - """Add timeout option to command.""" - return click.option( - "--timeout", - "timeout", - type=click.INT, - expose_value=False, - default=DEFAULT_RETRY_TIMEOUT, - show_default=True, - callback=update_global_conf, - help="Max number of seconds the operation will be retried for", - )(f) - - -def _format_server_errors(fn, errors): - action = fn.__name__.replace("_", " ") - - def _format_error_lines(errors_): - lines_ = [] - for field, field_errors in errors_.items(): - for field_error in field_errors: - lines_.append(f"{field}: {field_error}") - return lines_ - - lines = [] - if isinstance(errors, dict): - lines += _format_error_lines(errors) - elif isinstance(errors, list): - for error in errors: - lines += _format_error_lines(error) - else: - lines.append(errors) - - pluralized_error = "errors" if len(lines) > 1 else "error" - return ( - f"Could not {action}, the server returned the following \ - {pluralized_error}:\n- " - + "\n- ".join(lines) - ) - - -def error_printer(fn): - """Command decorator to pretty print a few selected exceptions from sdk.""" - - @functools.wraps(fn) - def wrapper(*args, **kwargs): - ctx = click.get_current_context() - if ctx.obj.verbose: - # disable pretty print of errors if verbose mode is activated - return fn(*args, **kwargs) - - try: - return fn(*args, **kwargs) - except exceptions.BadLoginException: - raise click.ClickException("Login failed: No active account found with the" " given credentials.") - except exceptions.InvalidRequest as e: - try: - errors = e.errors["message"] - except KeyError: - errors = e.errors - raise click.ClickException(_format_server_errors(fn, errors)) - except exceptions.RequestException as e: - raise click.ClickException(f"Request failed: {e.__class__.__name__}: {e}") - except ( - exceptions.ConnectionError, - exceptions.InvalidResponse, - exceptions.LoadDataException, - exceptions.BadConfiguration, - ) as e: - raise click.ClickException(str(e)) - - return wrapper - - -@click.group() -@click.version_option(__version__) -@click.pass_context -def cli(ctx): - """Substra Command Line Interface. - - For help using this tool, please open an issue on the Github repository: - https://github.com/Substra/substra - """ - ctx.obj = GlobalConf() - - -@cli.command("config") -@click.argument("url") -@click.option( - "--config", - type=click.Path(), - default=os.path.expanduser(configuration.DEFAULT_PATH), - help=f"Config path (default {configuration.DEFAULT_PATH}).", -) -@click.option("--profile", default="default", help="Profile name to add") -@click.option("--insecure", "-k", is_flag=True, help="Do not verify SSL certificates") -def add_profile_to_config(url, config, profile, insecure): - """Add profile to config file.""" - manager = configuration.ConfigManager(config) - manager.set_profile(name=profile, url=url, insecure=insecure) - manager.save() - - -@cli.command("login") -@click_global_conf -@click.pass_context -@error_printer -@click.option("--username", "-u", envvar="SUBSTRA_USERNAME", prompt=True) -@click.option("--password", "-p", envvar="SUBSTRA_PASSWORD", prompt=True, hide_input=True) -def login(ctx, username, password): - """Login to the Substra platform.""" - client = get_client(ctx.obj) - - token = client.login(username, password) - - # save token to tokens file - manager = configuration.TokenManager(ctx.obj.tokens) - manager.set_profile(ctx.obj.profile, token) - manager.save() - - display(f"Token: {token}") - - -@cli.group() -@click.pass_context -def organization(ctx): - """Display organization description.""" - - -@organization.command("info") -@click_global_conf_with_output_format -@click.pass_context -@error_printer -def organization_info(ctx): - """Display organization info.""" - client = get_client(ctx.obj) - res = client.organization_info() - printer = printers.OrganizationInfoPrinter() - printer.print(res) - - -@cli.group() -@click.pass_context -def cancel(ctx): - """Cancel execution of an asset.""" - pass - - -@cancel.command("compute_plan") -@click.argument("compute_plan_key", type=click.STRING) -@click_global_conf -@click.pass_context -def cancel_compute_plan(ctx, compute_plan_key): - """Cancel execution of a compute plan.""" - client = get_client(ctx.obj) - # method must exist in sdk - client.cancel_compute_plan(compute_plan_key) - - -if __name__ == "__main__": - cli() diff --git a/substra/cli/printers.py b/substra/cli/printers.py deleted file mode 100644 index e0455c90..00000000 --- a/substra/cli/printers.py +++ /dev/null @@ -1,134 +0,0 @@ -import enum -import json -import math - -import pydantic -import yaml - - -def find_dict_composite_key_value(asset_dict, composite_key): - def _recursive_find(d, keys): - value = d.get(keys[0]) - if len(keys) == 1: - return value - return _recursive_find(value or {}, keys[1:]) - - return _recursive_find(asset_dict, composite_key.split(".")) - - -class Field: - def __init__(self, name, ref): - self.name = name - self.ref = ref - - def get_value(self, item, expand=False): - value = find_dict_composite_key_value(item, self.ref) - if isinstance(value, enum.Enum): - value = value.name - return value - - def print_details(self, item, field_length, expand): - name = self.name.upper().ljust(field_length) - value = self.get_value(item, expand) - if isinstance(value, dict): - value = [f"{k}: {v}" for k, v in value.items()] - if isinstance(value, list): - if value: - print(name, end="") - padding = " " * field_length - for i, v in enumerate(value): - if i == 0: - print(f"- {v}") - else: - print(f"{padding}- {v}") - else: - print(f"{name}None") - else: - print(f"{name}{value}") - - -class MappingField(Field): - def get_value(self, item, expand=False): - mapping = super().get_value(item) or {} - if expand: - value = [f"{k}:{v}" for k, v in mapping.items()] - else: - value = f"{len(mapping)} values" - return value - - -class BasePrinter: - @staticmethod - def _get_columns(items, fields): - columns = [] - for field in fields: - values = [str(field.get_value(item)) for item in items] - - column = [field.name.upper()] - column.extend(values) - - columns.append(column) - return columns - - @staticmethod - def _get_column_widths(columns): - column_widths = [] - for column in columns: - width = max([len(x) for x in column]) - width = (math.ceil(width / 4) + 1) * 4 - column_widths.append(width) - return column_widths - - def print_table(self, items, fields): - columns = self._get_columns(items, fields) - column_widths = self._get_column_widths(columns) - - for row_index in range(len(items) + 1): - for col_index, column in enumerate(columns): - print(column[row_index].ljust(column_widths[col_index]), end="") - print() - - @staticmethod - def _get_field_name_length(fields): - max_length = max([len(field.name) for field in fields]) - length = (math.ceil(max_length / 4) + 1) * 4 - return length - - def print_details(self, item, fields, expand): - if isinstance(item, pydantic.BaseModel): - item = item.dict(exclude_none=False, by_alias=True) - field_length = self._get_field_name_length(fields) - for field in fields: - field.print_details(item, field_length, expand) - - -class JsonPrinter: - @staticmethod - def print(data, *args, **kwargs): - if isinstance(data, pydantic.BaseModel): - data = data.dict() - print(json.dumps(data, indent=2, default=str)) - - -class YamlPrinter: - @staticmethod - def print(data, *args, **kwargs): - # We need the yaml format to display the same things than json format - if isinstance(data, pydantic.BaseModel): - data = data.dict() - json_format = json.dumps(data, indent=2, default=str) - print(yaml.dump(json.loads(json_format), default_flow_style=False)) - - -class OrganizationInfoPrinter(BasePrinter): - single_fields = ( - Field("Host", "host"), - Field("Channel", "channel"), - Field("Backend version", "version"), - Field("Orchestrator version", "orchestrator_version"), - Field("Chaincode version", "chaincode_version"), - MappingField("Config", "config"), - ) - - def print(self, data): - self.print_details(data, self.single_fields, expand=True) diff --git a/substra/sdk/client.py b/substra/sdk/client.py index 6a9617a6..d2f14863 100644 --- a/substra/sdk/client.py +++ b/substra/sdk/client.py @@ -15,7 +15,6 @@ from slugify import slugify from substra.sdk import backends -from substra.sdk import config as cfg from substra.sdk import exceptions from substra.sdk import models from substra.sdk import schemas @@ -146,7 +145,7 @@ def get_client_configuration( """ if config_file and config_file.exists(): - config_dict = yaml.safe_load(config_file.read_text()) + config_dict = yaml.safe_load(config_file.read_text(encoding=None)) else: config_dict = {} @@ -361,59 +360,6 @@ def _get_spec(asset_type, data): return data return asset_type(**data) - @classmethod - def from_config_file( - cls, - profile_name: str = cfg.DEFAULT_PROFILE_NAME, - config_path: Union[str, pathlib.Path] = cfg.DEFAULT_PATH, - tokens_path: Union[str, pathlib.Path] = cfg.DEFAULT_TOKENS_PATH, - token: Optional[str] = None, - retry_timeout: int = DEFAULT_RETRY_TIMEOUT, - backend_type: schemas.BackendType = schemas.BackendType.REMOTE, - ): - """Returns a new Client configured with profile data from configuration files. - - Args: - profile_name (str, optional): Name of the profile to load. - Defaults to 'default'. - config_path (Union[str, pathlib.Path], optional): Path to the - configuration file. - Defaults to '~/.substra'. - tokens_path (Union[str, pathlib.Path], optional): Path to the tokens file. - Defaults to '~/.substra-tokens'. - token (str, optional): Token to use for authentication (will be used - instead of any token found at tokens_path). Defaults to None. - retry_timeout (int, optional): Number of seconds before attempting a retry call in case - of timeout. Defaults to 5 minutes. - backend_type (schemas.BackendType, optional): Which mode to use. - Possible values are `remote`, `docker` and `subprocess`. - Defaults to `remote`. - In `remote` mode, assets are registered on a deployed platform which also executes the tasks. - In `subprocess` or `docker` mode, if no URL is given then all assets are created locally and tasks are - executed locally. If a URL is given then the mode is a hybrid one: new assets are - created locally but can access assets from the deployed Substra platform. The platform is in read-only - mode and tasks are executed locally. - - Returns: - Client: The new client. - """ - - config_path = os.path.expanduser(config_path) - profile = cfg.ConfigManager(config_path).get_profile(profile_name) - if not token: - try: - tokens_path = os.path.expanduser(tokens_path) - token = cfg.TokenManager(tokens_path).get_profile(profile_name) - except cfg.ProfileNotFoundError: - token = None - return Client( - token=token, - retry_timeout=retry_timeout, - url=profile["url"], - insecure=profile["insecure"], - backend_type=backend_type, - ) - @logit def add_data_sample(self, data: Union[dict, schemas.DataSampleSpec], local: bool = True) -> str: """Create a new data sample asset and return its key. diff --git a/substra/sdk/config.py b/substra/sdk/config.py deleted file mode 100644 index 6d6771b7..00000000 --- a/substra/sdk/config.py +++ /dev/null @@ -1,79 +0,0 @@ -import abc -import json -import logging -import os - -logger = logging.getLogger(__name__) - -DEFAULT_PATH = "~/.substra" -DEFAULT_TOKENS_PATH = "~/.substra-tokens" -DEFAULT_PROFILE_NAME = "default" -DEFAULT_INSECURE = False - - -class ConfigException(Exception): - pass - - -class ProfileNotFoundError(ConfigException): - pass - - -class _ProfileManager(abc.ABC): - def __init__(self, path): - self._path = path - self._profiles = self.load() - - def load(self): - if not os.path.exists(self._path): - return {} - - with open(self._path) as fh: - try: - self._profiles = json.load(fh) - except json.decoder.JSONDecodeError: - raise ConfigException(f"Cannot parse config file '{self._path}'") - - return self._profiles - - def save(self): - with open(self._path, "w") as fh: - json.dump(self._profiles, fh, indent=2, sort_keys=True) - - def get_profile(self, name): - try: - return self._profiles[name] - except KeyError: - raise ProfileNotFoundError(name) - - def set_profile(self, name, profile): - if name in self._profiles: - self._profiles[name].update(profile) - else: - self._profiles[name] = profile - return self._profiles[name] - - -class ConfigManager(_ProfileManager): - def set_profile(self, name, url, insecure=False): - return super().set_profile( - name, - { - "url": url, - "insecure": insecure, - }, - ) - - -class TokenManager(_ProfileManager): - def set_profile(self, name, token): - return super().set_profile( - name, - { - "token": token, - }, - ) - - def get_profile(self, name): - profile = super().get_profile(name) - return profile["token"] diff --git a/tests/sdk/test_client.py b/tests/sdk/test_client.py index a44897ea..4464e2b2 100644 --- a/tests/sdk/test_client.py +++ b/tests/sdk/test_client.py @@ -1,6 +1,7 @@ import pytest import yaml +from substra.sdk import exceptions from substra.sdk import schemas from substra.sdk.client import Client from substra.sdk.client import _upper_slug @@ -193,3 +194,8 @@ def test_client_configuration_env_var_overrides_config_file(mocker, monkeypatch, assert client._token == "env_var_token" assert client._retry_timeout == 12 assert client._insecure is False + + +def test_login_remote_without_url(tmpdir): + with pytest.raises(exceptions.SDKException): + Client(backend_type="remote") diff --git a/tests/sdk/test_config.py b/tests/sdk/test_config.py deleted file mode 100644 index 77bbcba0..00000000 --- a/tests/sdk/test_config.py +++ /dev/null @@ -1,109 +0,0 @@ -import json - -import pytest - -import substra -import substra.sdk.config as configuration - -DUMMY_CONFIG = { - configuration.DEFAULT_PROFILE_NAME: { - "url": "http://example.com", - "insecure": configuration.DEFAULT_INSECURE, - } -} - -DUMMY_TOKENS = { - configuration.DEFAULT_PROFILE_NAME: { - "token": "foo", - } -} - - -def test_add_from_config_file(tmpdir): - path = tmpdir / "substra.cfg" - manager_1 = configuration.ConfigManager(str(path)) - - profile_1 = manager_1.set_profile("owkin", url="http://substra-backend.owkin.xyz:8000") - manager_1.save() - - manager_2 = configuration.ConfigManager(str(path)) - profile_2 = manager_2.get_profile("owkin") - assert profile_1 == profile_2 - - -def test_add_from_config_file_from_file(tmpdir): - path = tmpdir / "substra.cfg" - conf = { - "organization-1": { - "insecure": False, - "url": "http://substra-backend.org-1.com", - }, - } - - path.write_text(json.dumps(conf), "UTF-8") - manager = configuration.ConfigManager(str(path)) - profile = manager.get_profile("organization-1") - - assert conf["organization-1"] == profile - - -def test_add_load_bad_profile_from_file(tmpdir): - path = tmpdir / "substra.cfg" - conf = "organization-1" - - path.write_text(conf, "UTF-8") - - with pytest.raises(configuration.ConfigException): - configuration.ConfigManager(str(path)) - - -def test_from_config_file_fail(tmpdir): - path = tmpdir / "substra.cfg" - manager = configuration.ConfigManager(str(path)) - - with pytest.raises(configuration.ConfigException): - manager.get_profile("notfound") - - manager.set_profile("default", "foo", "bar") - - with pytest.raises(configuration.ProfileNotFoundError): - manager.get_profile("notfound") - - -def test_login_remote_without_url(tmpdir): - with pytest.raises(substra.exceptions.SDKException): - substra.Client(backend_type="remote") - - -def test_token_without_tokens_path(tmpdir): - config_path = tmpdir / "substra.json" - config_path.write_text(json.dumps(DUMMY_CONFIG), "UTF-8") - - tokens_path = tmpdir / "substra-tokens.json" - - client = substra.Client.from_config_file(config_path=config_path, profile_name="default") - assert client._token == "" - - client = substra.Client.from_config_file( - config_path=config_path, tokens_path=tokens_path, profile_name="default", token="foo" - ) - assert client._token == "foo" - - -def test_token_with_tokens_path(tmpdir): - config_path = tmpdir / "substra.json" - config_path.write_text(json.dumps(DUMMY_CONFIG), "UTF-8") - - tokens_path = tmpdir / "substra-tokens.json" - tokens_path.write_text(json.dumps(DUMMY_TOKENS), "UTF-8") - - client = substra.Client.from_config_file(config_path=config_path, profile_name="default", tokens_path=tokens_path) - assert client._token == "foo" - - client = substra.Client.from_config_file( - config_path=config_path, - profile_name="default", - tokens_path=tokens_path, - token="bar", - ) - assert client._token == "bar" diff --git a/tests/test_cli.py b/tests/test_cli.py deleted file mode 100644 index 264e1799..00000000 --- a/tests/test_cli.py +++ /dev/null @@ -1,111 +0,0 @@ -import json -import sys -import traceback -from unittest import mock - -import click -import pytest -from click.testing import CliRunner - -import substra -from substra.cli.interface import cli -from substra.cli.interface import error_printer - - -def execute(command, exit_code=0): - runner = CliRunner() - result = runner.invoke(cli, command) - if exit_code == 0 and result.exit_code != 0 and result.exc_info: - # if the test was supposed to succeed but failed, - # we display the exception and the full traceback - _, _, tb = result.exc_info - traceback.print_tb(tb, file=sys.stdout) - pytest.fail(str(result.exception)) - assert result.exit_code == exit_code, result.output - return result.output - - -def client_execute(directory, command, exit_code=0): - # force using a new config file and a new profile - if "--config" not in command: - cfgpath = directory / "substra.cfg" - manager = substra.sdk.config.ConfigManager(str(cfgpath)) - manager.set_profile("default", url="http://foo") - manager.save() - command.extend(["--config", str(cfgpath)]) - return execute(command, exit_code=exit_code) - - -def test_command_help(): - output = execute(["--help"]) - assert "Usage:" in output - - -def test_command_version(): - output = execute(["--version"]) - assert substra.__version__ in output - - -def test_command_config(workdir): - cfgfile = workdir / "cli.cfg" - - assert cfgfile.exists() is False - - new_url = "http://foo" - new_profile = "foo" - execute( - [ - "config", - new_url, - "--profile", - new_profile, - "--config", - str(cfgfile), - ] - ) - assert cfgfile.exists() - - # check new profile has been created, check also that default profile - # has been created - with cfgfile.open() as fp: - cfg = json.load(fp) - expected_profiles = ["foo"] - assert list(cfg.keys()) == expected_profiles - - -def mock_client_call(mocker, method_name, response="", side_effect=None): - return mocker.patch(f"substra.cli.interface.Client.{method_name}", return_value=response, side_effect=side_effect) - - -def test_command_login(workdir, mocker): - m = mock_client_call(mocker, "login") - client_execute(workdir, ["login", "--username", "foo", "--password", "bar"]) - m.assert_called() - - -def test_command_cancel_compute_plan(workdir, mocker): - m = mock_client_call(mocker, "cancel_compute_plan", None) - client_execute(workdir, ["cancel", "compute_plan", "fakekey"]) - m.assert_called() - - -@pytest.mark.parametrize( - "exception", - [ - (substra.exceptions.RequestException("foo", 400)), - (substra.exceptions.ConnectionError("foo", 400)), - (substra.exceptions.InvalidResponse(None, "foo")), - (substra.exceptions.LoadDataException("foo", 400)), - ], -) -def test_error_printer(mocker, exception): - @error_printer - def foo(): - raise exception - - mock_click_context = mock.MagicMock() - mock_click_context.obj.verbose = False - - mocker.patch("substra.cli.interface.click.get_current_context", return_value=mock_click_context) - with pytest.raises(click.ClickException, match="foo"): - foo() diff --git a/tests/test_cli_integration.py b/tests/test_cli_integration.py deleted file mode 100644 index b3841782..00000000 --- a/tests/test_cli_integration.py +++ /dev/null @@ -1,13 +0,0 @@ -""" - Test the cli and its call to the sdk. Mock the backend instead of the sdk. - """ - - -from tests import mocked_requests -from tests.test_cli import client_execute - - -def test_cancel_compute_plan(workdir, mocker): - m = mocked_requests.cancel_compute_plan(mocker) - client_execute(workdir, ["cancel", "compute_plan", "fakekey"]) - m.assert_called() diff --git a/tests/test_printers.py b/tests/test_printers.py deleted file mode 100644 index 0532c5a1..00000000 --- a/tests/test_printers.py +++ /dev/null @@ -1,22 +0,0 @@ -import pytest - -from substra.cli import printers - - -@pytest.mark.parametrize( - "obj,path,res", - [ - ({}, "a", None), - ({}, "a.b", None), - ({"a": None}, "a.b", None), - ({"a": "a"}, "a", "a"), - ({"a": {"b": "b"}}, "a.b", "b"), - ], -) -def test_find_dict_composite_key_value(obj, path, res): - assert printers.find_dict_composite_key_value(obj, path) == res - - -def test_find_dict_composite_key_value_fails(): - with pytest.raises(AttributeError): - printers.find_dict_composite_key_value({"a": "b"}, "a.b")