Changeset View
Changeset View
Standalone View
Standalone View
swh/scanner/cli.py
Show All 12 Lines | |||||
from swh.core import config | from swh.core import config | ||||
from swh.core.cli import CONTEXT_SETTINGS | from swh.core.cli import CONTEXT_SETTINGS | ||||
from swh.core.cli import swh as swh_cli_group | from swh.core.cli import swh as swh_cli_group | ||||
# All generic config code should reside in swh.core.config | # All generic config code should reside in swh.core.config | ||||
CONFIG_ENVVAR = "SWH_CONFIG_FILE" | CONFIG_ENVVAR = "SWH_CONFIG_FILE" | ||||
DEFAULT_CONFIG_PATH = os.path.join(click.get_app_dir("swh"), "global.yml") | DEFAULT_CONFIG_PATH = os.path.join(click.get_app_dir("swh"), "global.yml") | ||||
DEFAULT_PATH = os.environ.get(CONFIG_ENVVAR, DEFAULT_CONFIG_PATH) | |||||
DEFAULT_CONFIG: Dict[str, Any] = { | DEFAULT_CONFIG: Dict[str, Any] = { | ||||
"web-api": { | "web-api": { | ||||
"url": "https://archive.softwareheritage.org/api/1/", | "url": "https://archive.softwareheritage.org/api/1/", | ||||
"auth-token": None, | "auth-token": None, | ||||
} | } | ||||
} | } | ||||
Show All 22 Lines | @click.option( | ||||
"-C", | "-C", | ||||
"--config-file", | "--config-file", | ||||
default=None, | default=None, | ||||
type=click.Path(exists=False, dir_okay=False, path_type=str), | type=click.Path(exists=False, dir_okay=False, path_type=str), | ||||
help="""YAML configuration file""", | help="""YAML configuration file""", | ||||
) | ) | ||||
@click.pass_context | @click.pass_context | ||||
def scanner(ctx, config_file: Optional[str]): | def scanner(ctx, config_file: Optional[str]): | ||||
if config_file is None and config.config_exists(DEFAULT_PATH): | |||||
config_file = DEFAULT_PATH | |||||
douardda: This `def_path` var is useless | |||||
if config_file is None: | env_config_path = os.environ.get(CONFIG_ENVVAR) | ||||
conf = DEFAULT_CONFIG | |||||
Done Inline ActionsThis comment in not really useful either. It gives the impression we plan on having a dynamic config system, which we do not. douardda: This comment in not really useful either. It gives the impression we plan on having a dynamic… | |||||
else: | # read_raw_config do not fail if file does not exist, so check it beforehand | ||||
Not Done Inline Actionswhy is this if statement not in the if not config_file below? If the env var is set but is dandling pointer but the config_file file has been explicitly given, we should not fail IMHO. douardda: why is this if statement not in the `if not config_file` below? If the env var is set but is… | |||||
# read_raw_config do not fail on ENOENT | # while enforcing loading priority | ||||
if config_file: | |||||
if not config.config_exists(config_file): | if not config.config_exists(config_file): | ||||
raise FileNotFoundError(config_file) | raise click.BadParameter( | ||||
Done Inline Actionsthe elif are a bit disturbing here. I also find that this whole snip of code does not really make explicit and clear the logic behind it. I'd rather write it like if not config_file: if env_path: config_file = env_path if config_file: if not config.config_exists(config_file): raise FileNotFound() else: if config.config_exists(DEFAULT_CONFIG_PATH): config_file = DEFAULT_CONFIG_PATH It's not shorter but it expresses the logic better IMHO. douardda: the elif are a bit disturbing here.
I also find that this whole snip of code does not really… | |||||
f"File '{config_file}' cannot be opened.", param_hint="--config-file" | |||||
) | |||||
elif env_config_path: | |||||
if not config.config_exists(env_config_path): | |||||
raise click.BadParameter( | |||||
f"File '{env_config_path}' cannot be opened.", param_hint=CONFIG_ENVVAR | |||||
) | |||||
config_file = env_config_path | |||||
Done Inline Actionschoose either a syntax or the other, but do not let this second option a comment. douardda: choose either a syntax or the other, but do not let this second option a comment. | |||||
elif config.config_exists(DEFAULT_CONFIG_PATH): | |||||
config_file = DEFAULT_CONFIG_PATH | |||||
conf = DEFAULT_CONFIG | |||||
if config_file is not None: | |||||
conf = config.read_raw_config(config.config_basepath(config_file)) | conf = config.read_raw_config(config.config_basepath(config_file)) | ||||
conf = config.merge_configs(DEFAULT_CONFIG, conf) | conf = config.merge_configs(DEFAULT_CONFIG, conf) | ||||
ctx.ensure_object(dict) | ctx.ensure_object(dict) | ||||
ctx.obj["config"] = conf | ctx.obj["config"] = conf | ||||
@scanner.command(name="scan") | @scanner.command(name="scan") | ||||
Show All 26 Lines | |||||
) | ) | ||||
@click.option( | @click.option( | ||||
"-i", "--interactive", is_flag=True, help="Show the result in a dashboard" | "-i", "--interactive", is_flag=True, help="Show the result in a dashboard" | ||||
) | ) | ||||
@click.pass_context | @click.pass_context | ||||
def scan(ctx, root_path, api_url, patterns, out_fmt, interactive): | def scan(ctx, root_path, api_url, patterns, out_fmt, interactive): | ||||
"""Scan a source code project to discover files and directories already | """Scan a source code project to discover files and directories already | ||||
present in the archive""" | present in the archive""" | ||||
from .scanner import scan | import swh.scanner.scanner as scanner | ||||
config = ctx.obj["config"] | config = ctx.obj["config"] | ||||
if api_url: | if api_url: | ||||
if not api_url.endswith("/"): | if not api_url.endswith("/"): | ||||
api_url += "/" | api_url += "/" | ||||
config["web-api"]["url"] = api_url | config["web-api"]["url"] = api_url | ||||
scan(config, root_path, patterns, out_fmt, interactive) | scanner.scan(config, root_path, patterns, out_fmt, interactive) | ||||
def main(): | def main(): | ||||
return scanner(auto_envvar_prefix="SWH_SCANNER") | return scanner(auto_envvar_prefix="SWH_SCANNER") | ||||
if __name__ == "__main__": | if __name__ == "__main__": | ||||
main() | main() |
This def_path var is useless