diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 5cd75f42be..b1179794c6 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -5,3 +5,4 @@ ## Refactoring * #800: Removed tbx security pretty-print, tbx lint pretty-print, and creation of .lint.txt, as superseded by Sonar and .lint.json usage +* #791: Resolved Sonar concerns: accepted specific `subprocess` import usage & improved minor maintainability items diff --git a/exasol/toolbox/nox/_artifacts.py b/exasol/toolbox/nox/_artifacts.py index 19eab7d9da..fe28599c61 100644 --- a/exasol/toolbox/nox/_artifacts.py +++ b/exasol/toolbox/nox/_artifacts.py @@ -2,7 +2,7 @@ import os import shutil import sqlite3 -import subprocess # nosec +import subprocess # nosec: B404 - risk of subprocess is accepted import sys from collections.abc import Iterable from pathlib import Path @@ -42,13 +42,13 @@ def check_artifacts(session: Session) -> None: """Validate that all project artifacts are available and consistent""" all_files = {f.name for f in PROJECT_CONFIG.root_path.iterdir() if f.is_file()} if missing_files := (ALL_LINT_FILES - all_files): - print(f"files not available: {missing_files}", file=sys.stderr) + print(f"files not available: {sorted(missing_files)}", file=sys.stderr) sys.exit(1) all_is_valid_checks = [ + _is_valid_coverage(Path(PROJECT_CONFIG.root_path, COVERAGE_DB)), _is_valid_lint_json(Path(PROJECT_CONFIG.root_path, LINT_JSON)), _is_valid_security_json(Path(PROJECT_CONFIG.root_path, SECURITY_JSON)), - _is_valid_coverage(Path(PROJECT_CONFIG.root_path, COVERAGE_DB)), ] if not all(all_is_valid_checks): sys.exit(1) diff --git a/exasol/toolbox/nox/_documentation.py b/exasol/toolbox/nox/_documentation.py index 4d4b106d75..5c2988e0d3 100644 --- a/exasol/toolbox/nox/_documentation.py +++ b/exasol/toolbox/nox/_documentation.py @@ -3,7 +3,7 @@ import argparse import json import shutil -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted import sys import tempfile import webbrowser diff --git a/exasol/toolbox/nox/_release.py b/exasol/toolbox/nox/_release.py index 1607292205..ad50104ce5 100644 --- a/exasol/toolbox/nox/_release.py +++ b/exasol/toolbox/nox/_release.py @@ -2,7 +2,7 @@ import argparse import re -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted from pathlib import Path import nox diff --git a/exasol/toolbox/sphinx/multiversion/git.py b/exasol/toolbox/sphinx/multiversion/git.py index 4737ee038a..22fca61259 100644 --- a/exasol/toolbox/sphinx/multiversion/git.py +++ b/exasol/toolbox/sphinx/multiversion/git.py @@ -3,7 +3,7 @@ import logging import os import re -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted import tarfile import tempfile @@ -143,7 +143,7 @@ def file_exists(gitroot, refname, filename): return proc.returncode == 0 -def copy_tree(gitroot, src, dst, reference, sourcepath="."): +def copy_tree(gitroot, dst, reference, sourcepath="."): with tempfile.SpooledTemporaryFile() as fp: cmd = ( "git", diff --git a/exasol/toolbox/sphinx/multiversion/main.py b/exasol/toolbox/sphinx/multiversion/main.py index f183d97cc8..b3fea3760c 100644 --- a/exasol/toolbox/sphinx/multiversion/main.py +++ b/exasol/toolbox/sphinx/multiversion/main.py @@ -10,7 +10,7 @@ import re import shutil import string -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted import sys import tempfile from importlib import resources @@ -310,7 +310,7 @@ def _main(args, argv): # Clone Git repo repopath = os.path.join(tmp, gitref.commit) try: - git.copy_tree(str(gitroot), gitroot.as_uri(), repopath, gitref) + git.copy_tree(str(gitroot), repopath, gitref) except (OSError, subprocess.CalledProcessError): logger.error( "Failed to copy git tree for %s to %s", @@ -524,9 +524,8 @@ def _main(args, argv): build_artefacts = candidate_files if len(build_artefacts) == 0: logger.warning( - ("Build artefact {project}" "not found.").format( - project=build_file_pattern.lower(), - ) + "Build artifact %s not found.", + build_file_pattern.lower(), ) elif len(build_artefacts) > 1: logger.warning( diff --git a/exasol/toolbox/tools/issue.py b/exasol/toolbox/tools/issue.py index a41658841e..2eb35efdef 100644 --- a/exasol/toolbox/tools/issue.py +++ b/exasol/toolbox/tools/issue.py @@ -7,6 +7,7 @@ CLI = typer.Typer() PKG = "exasol.toolbox.templates.github.ISSUE_TEMPLATE" LEXER = "markdown" +ISSUE_TEMPLATE_PATH = Path("./.github/ISSUE_TEMPLATE") @CLI.command(name="list") @@ -31,7 +32,7 @@ def show_issue( def diff_issue( issue: str = typer.Argument(..., help="issue which shall be diffed."), dest: Path = typer.Argument( - Path("./.github/ISSUE_TEMPLATE"), + ISSUE_TEMPLATE_PATH, help="target directory to diff the issue against.", ), ) -> None: @@ -47,7 +48,7 @@ def diff_issue( def install_issue( issue: str = typer.Argument("all", help="name of the issue to install."), dest: Path = typer.Argument( - Path("./.github/ISSUE_TEMPLATE"), + ISSUE_TEMPLATE_PATH, help="target directory to install the issue to.", ), ) -> None: @@ -63,7 +64,7 @@ def install_issue( def update_issue( issue: str = typer.Argument("all", help="name of the issue to install."), dest: Path = typer.Argument( - Path("./.github/ISSUE_TEMPLATE"), + ISSUE_TEMPLATE_PATH, help="target directory to install the issue to.", ), confirm: bool = typer.Option( diff --git a/exasol/toolbox/tools/security.py b/exasol/toolbox/tools/security.py index 9fc30161b4..bcc5e0017d 100644 --- a/exasol/toolbox/tools/security.py +++ b/exasol/toolbox/tools/security.py @@ -4,7 +4,7 @@ import json import re -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted import sys from collections.abc import ( Generator, @@ -159,7 +159,7 @@ def from_pip_audit(report: str) -> Iterable[Issue]: ) if cves: yield Issue( - cve=sorted(cves)[0], + cve=min(cves), cwe="None" if not cwes else ", ".join(cwes), description=vulnerability["description"], coordinates=vulnerability["coordinates"], diff --git a/exasol/toolbox/util/dependencies/audit.py b/exasol/toolbox/util/dependencies/audit.py index 26bc4fbd38..553c835f73 100644 --- a/exasol/toolbox/util/dependencies/audit.py +++ b/exasol/toolbox/util/dependencies/audit.py @@ -1,7 +1,7 @@ from __future__ import annotations import json -import subprocess # nosec +import subprocess # nosec: B404 - risk of subprocess is accepted import tempfile from dataclasses import dataclass from enum import Enum diff --git a/exasol/toolbox/util/dependencies/licenses.py b/exasol/toolbox/util/dependencies/licenses.py index db329e9076..d40d9d91ad 100644 --- a/exasol/toolbox/util/dependencies/licenses.py +++ b/exasol/toolbox/util/dependencies/licenses.py @@ -1,6 +1,6 @@ from __future__ import annotations -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted import tempfile from collections import OrderedDict from dataclasses import dataclass diff --git a/exasol/toolbox/util/dependencies/poetry_dependencies.py b/exasol/toolbox/util/dependencies/poetry_dependencies.py index e56ecefe0f..26a0265565 100644 --- a/exasol/toolbox/util/dependencies/poetry_dependencies.py +++ b/exasol/toolbox/util/dependencies/poetry_dependencies.py @@ -1,6 +1,6 @@ from __future__ import annotations -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted from collections import OrderedDict from pathlib import Path diff --git a/exasol/toolbox/util/git.py b/exasol/toolbox/util/git.py index e74ab640c9..0322bc4bbc 100644 --- a/exasol/toolbox/util/git.py +++ b/exasol/toolbox/util/git.py @@ -1,4 +1,4 @@ -import subprocess # nosec +import subprocess # nosec: B404 - risk of subprocess is accepted from functools import wraps from pathlib import Path diff --git a/exasol/toolbox/util/version.py b/exasol/toolbox/util/version.py index d65968384d..86dc177e2d 100644 --- a/exasol/toolbox/util/version.py +++ b/exasol/toolbox/util/version.py @@ -1,6 +1,6 @@ from __future__ import annotations -import subprocess +import subprocess # nosec: B404 - risk of subprocess is accepted from dataclasses import dataclass from enum import Enum from functools import ( diff --git a/test/unit/nox/_artifacts_test.py b/test/unit/nox/_artifacts_test.py index e086ded340..b850e3b6a5 100644 --- a/test/unit/nox/_artifacts_test.py +++ b/test/unit/nox/_artifacts_test.py @@ -118,7 +118,9 @@ def test_fails_when_file_missing( ) as session: with pytest.raises(SystemExit): check_artifacts(session) - assert f"files not available: {missing_files}" in capsys.readouterr().err + assert ( + f"files not available: {sorted(missing_files)}" in capsys.readouterr().err + ) def test_fails_when_check_fails( self, test_project_config_factory, tmp_path, capsys