From c3a4e7176834fa91c264adaae4a327b294bd550c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Berland?= Date: Fri, 6 Jan 2023 15:30:15 +0100 Subject: [PATCH] Add pylint to ci Currently every current pylint issue is exempted. Excemptions (disables) in .pylintrc are to be removed one-by-one in upcoming PRs. Solve selected pylint issues --- komodo/build.py | 8 ++++---- komodo/cli.py | 3 +-- komodo/fetch.py | 6 +++--- komodo/insert_proposals.py | 12 +++++------- komodo/lint.py | 3 ++- komodo/prettier.py | 2 +- komodo/release_transpiler.py | 12 ++++++------ komodo/reverse_dep_graph.py | 6 +++--- komodo/snyk_reporting.py | 12 ++++-------- setup.cfg | 11 ----------- tests/test_insert_proposals.py | 7 +++++-- tests/test_link_io_structure.py | 2 +- tests/test_lint_package_status.py | 2 +- tests/test_post_messages.py | 4 ++-- 14 files changed, 38 insertions(+), 52 deletions(-) diff --git a/komodo/build.py b/komodo/build.py index a9f793bfd..043c07fda 100644 --- a/komodo/build.py +++ b/komodo/build.py @@ -70,8 +70,8 @@ def cmake( builddir, makeopts, jobs, - cmake="cmake", *args, + cmake="cmake", **kwargs, ): bdir = f"{pkg}-{ver}-build" @@ -150,7 +150,7 @@ def download(pkg, ver, pkgpath, data, prefix, *args, **kwargs): if not url.startswith("https"): raise ValueError(f"{url} does not use https:// protocol") - hash_type, hash = kwargs["hash"].split(":") + hash_type, hash_value = kwargs["hash"].split(":") if hash_type != "sha256": raise NotImplementedError( f"Hash type {hash_type} given - only sha256 implemented" @@ -175,7 +175,7 @@ def download(pkg, ver, pkgpath, data, prefix, *args, **kwargs): file_handle.write(chunk) sha256.update(chunk) - if sha256.hexdigest() != hash: + if sha256.hexdigest() != hash_value: raise ValueError( f"Hash of downloaded file ({sha256.hexdigest()}) " "not equal to expected hash." @@ -188,7 +188,7 @@ def download(pkg, ver, pkgpath, data, prefix, *args, **kwargs): ) -def pip_install(pkg, ver, pkgpath, data, prefix, dlprefix, pip="pip", *args, **kwargs): +def pip_install(pkg, ver, pkgpath, data, prefix, dlprefix, *args, pip="pip", **kwargs): ver = strip_version(ver) if ver == LATEST_PACKAGE_ALIAS: ver = latest_pypi_version(pkg) diff --git a/komodo/cli.py b/komodo/cli.py index 670e9b897..5c0e0f172 100755 --- a/komodo/cli.py +++ b/komodo/cli.py @@ -5,8 +5,7 @@ import yaml as yml -import komodo.local as local -import komodo.switch as switch +from komodo import local, switch from komodo.build import make from komodo.data import Data from komodo.fetch import fetch diff --git a/komodo/fetch.py b/komodo/fetch.py index d007905f7..5c73aac86 100644 --- a/komodo/fetch.py +++ b/komodo/fetch.py @@ -47,7 +47,7 @@ def grab(path, filename=None, version=None, protocol=None, pip="pip"): raise NotImplementedError(f"Unknown protocol {protocol}") -def fetch(pkgs, repo, outdir, pip="pip"): +def fetch(pkgs, repo, outdir, pip="pip") -> dict: missingpkg = [pkg for pkg in pkgs if pkg not in repo] missingver = [ pkg for pkg, ver in pkgs.items() if pkg in repo and ver not in repo[pkg] @@ -64,14 +64,14 @@ def fetch(pkgs, repo, outdir, pip="pip"): ) if missingpkg or missingver: - return + return {} if not outdir: raise ValueError( "The value of `outdir`, the cache location for pip and other " "tools, cannot be None or the empty string." ) - elif not os.path.exists(outdir): + if not os.path.exists(outdir): os.mkdir(outdir) pypi_packages = [] diff --git a/komodo/insert_proposals.py b/komodo/insert_proposals.py index 6d0c401cf..3dd473901 100644 --- a/komodo/insert_proposals.py +++ b/komodo/insert_proposals.py @@ -69,14 +69,12 @@ def load_yaml_from_repo(filename, repo, ref): def main(): args = parse_args() repo = _get_repo(os.getenv("GITHUB_TOKEN"), args.git_fork, args.git_repo) - status = insert_proposals( + insert_proposals( repo, args.base, args.target, args.git_ref, args.jobname, args.joburl ) - if status is not None: - raise status -def insert_proposals(repo, base, target, git_ref, jobname, joburl): +def insert_proposals(repo, base, target, git_ref, jobname, joburl) -> None: year = target.split(".")[0] month = target.split(".")[1] tmp_target = target + ".tmp" @@ -87,21 +85,21 @@ def insert_proposals(repo, base, target, git_ref, jobname, joburl): except github.GithubException: pass else: - return ValueError(f"Branch {target} exists already") + raise ValueError(f"Branch {target} exists already") try: repo.get_branch(tmp_target) except github.GithubException: pass else: - return ValueError(f"Branch {tmp_target} exists already") + raise ValueError(f"Branch {tmp_target} exists already") # create contents of new release proposal_yaml = load_yaml_from_repo("upgrade_proposals.yml", repo, git_ref) upgrade_key = f"{year}-{month}" upgrade = proposal_yaml.get(upgrade_key) if upgrade_key not in proposal_yaml: - return ValueError( + raise ValueError( f"No section for this release ({upgrade_key}) in upgrade_proposals.yml" ) base_file = f"releases/matrices/{base}.yml" diff --git a/komodo/lint.py b/komodo/lint.py index 8acf89f4c..09c271938 100755 --- a/komodo/lint.py +++ b/komodo/lint.py @@ -94,7 +94,8 @@ def lint_version_numbers(pkgs, repo): # A warning coincides with finding "Legacy" in repr(v) if "Legacy" in repr(v): # don't know if possible to check otherwise __reg_version_err(errs, pkg, ver, maintainer) - except: # noqa + except: # pylint: disable=bare-except # noqa + # Log any exception: __reg_version_err(errs, pkg, ver, maintainer) return errs diff --git a/komodo/prettier.py b/komodo/prettier.py index 324858590..0960dd5a8 100644 --- a/komodo/prettier.py +++ b/komodo/prettier.py @@ -116,7 +116,7 @@ def prettified_yaml(filepath, check_only=True): def write_to_string(repository, check_type=True): - if type(repository) == dict: + if isinstance(repository, dict): repository = ordereddict(sorted(repository.items(), key=lambda t: t[0])) repository = ruamel.yaml.comments.CommentedMap(repository) return prettier(repository, check_type) diff --git a/komodo/release_transpiler.py b/komodo/release_transpiler.py index b4ffd7212..85670893c 100755 --- a/komodo/release_transpiler.py +++ b/komodo/release_transpiler.py @@ -19,15 +19,15 @@ def build_matrix_file(release_base, release_folder, builtins): ) compiled = {} - for p in all_packages: - if p in builtins: - compiled[p] = builtins[p] + for package in all_packages: + if package in builtins: + compiled[package] = builtins[package] continue - if len(set([files[key].get(p) for key in files])) == 1: - compiled[p] = next(iter(files.values()))[p] + if len({files[key].get(package) for key in files}) == 1: + compiled[package] = next(iter(files.values()))[package] else: - compiled[p] = {key: files[key].get(p) for key in py_keys} + compiled[package] = {key: files[key].get(package) for key in py_keys} write_to_file(compiled, f"{release_base}.yml", False) diff --git a/komodo/reverse_dep_graph.py b/komodo/reverse_dep_graph.py index 17a11403c..1694e0abb 100644 --- a/komodo/reverse_dep_graph.py +++ b/komodo/reverse_dep_graph.py @@ -58,13 +58,13 @@ def _dump_dot(reverse, pkg, version, out): def _dump_dot_dep(reverse, pkg, version, out, seen): if pkg in seen: return - id = pkg.lower().replace("-", "_") - out.write(f' {id} [label="{pkg}-{version}"];\n') + _id = pkg.lower().replace("-", "_") + out.write(f' {_id} [label="{pkg}-{version}"];\n') if pkg in reverse: seen.add(pkg) for rev_dep, rev_version in reverse[pkg]: rev_id = rev_dep.lower().replace("-", "_") - out.write(f" {id} -> {rev_id};\n") + out.write(f" {_id} -> {rev_id};\n") _dump_dot_dep(reverse, rev_dep, rev_version, out, seen) diff --git a/komodo/snyk_reporting.py b/komodo/snyk_reporting.py index e31ece97c..7f3a6c8c6 100644 --- a/komodo/snyk_reporting.py +++ b/komodo/snyk_reporting.py @@ -4,13 +4,9 @@ import sys from typing import Any, Dict, List -if sys.version_info >= (3, 7): - from snyk import SnykClient - from snyk.managers import OrganizationManager - from snyk.models import Vulnerability -else: - OrganizationManager = Any - Vulnerability = Any +from snyk import SnykClient +from snyk.managers import OrganizationManager +from snyk.models import Vulnerability from komodo.yaml_file_type import ReleaseDir, ReleaseFile, YamlFile @@ -89,7 +85,7 @@ def find_vulnerabilities( repository: Dict[str, Any], org: OrganizationManager, ) -> Dict[str, List[Vulnerability]]: - result = dict() + result = {} for release_name, packages in releases.items(): pip_packages = filter_pip_packages(packages=packages, repository=repository) diff --git a/setup.cfg b/setup.cfg index ea560f1c5..62598d3ee 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,12 +3,9 @@ max-line-length = 88 [pylint.message] disable = bad-inline-option, - bare-except, broad-except, consider-using-dict-items, consider-using-f-string, - consider-using-from-import, - consider-using-set-comprehension, consider-using-sys-exit, consider-using-with, deprecated-pragma, @@ -16,23 +13,18 @@ disable = bad-inline-option, expression-not-assigned, file-ignored, fixme, - inconsistent-return-statements, invalid-name, - keyword-arg-before-vararg, line-too-long, - literal-comparison, locally-disabled, logging-not-lazy, missing-class-docstring, missing-function-docstring, missing-module-docstring, missing-timeout, - no-else-raise, no-else-return, protected-access, raise-missing-from, raw-checker-failed, - redefined-builtin, redefined-outer-name, suppressed-message, too-few-public-methods, @@ -40,7 +32,6 @@ disable = bad-inline-option, too-many-branches, too-many-locals, too-many-statements, - unidiomatic-typecheck, unnecessary-comprehension, unnecessary-lambda, unnecessary-pass, @@ -49,9 +40,7 @@ disable = bad-inline-option, unused-import, unused-variable, use-a-generator, - use-dict-literal, use-implicit-booleaness-not-comparison, use-symbolic-message-instead, used-before-assignment, useless-object-inheritance, - wrong-import-position, diff --git a/tests/test_insert_proposals.py b/tests/test_insert_proposals.py index 1369ae40e..2665ff014 100644 --- a/tests/test_insert_proposals.py +++ b/tests/test_insert_proposals.py @@ -148,9 +148,12 @@ def create_pull(self, title, body, head, base): ) def test_insert_proposals(base, target, repo_files, changed_files, prs, return_type): repo = MockRepo(files=repo_files) - status = insert_proposals(repo, base, target, "git_ref", "jobname", "joburl") - assert isinstance(status, return_type) + if isinstance(return_type(), Exception): + with pytest.raises(return_type): + insert_proposals(repo, base, target, "git_ref", "jobname", "joburl") + else: + insert_proposals(repo, base, target, "git_ref", "jobname", "joburl") assert len(changed_files) == len(repo.updated_files) for file, content in changed_files.items(): diff --git a/tests/test_link_io_structure.py b/tests/test_link_io_structure.py index 11f6f0806..28fe7850d 100644 --- a/tests/test_link_io_structure.py +++ b/tests/test_link_io_structure.py @@ -41,7 +41,7 @@ def test_read_folder_structure(tmpdir): output_dict = read_link_structure(os.getcwd()) - assert type(output_dict) == dict + assert isinstance(output_dict, dict) assert equal_links(output_dict, expected_result) diff --git a/tests/test_lint_package_status.py b/tests/test_lint_package_status.py index d4495e404..180fc5149 100644 --- a/tests/test_lint_package_status.py +++ b/tests/test_lint_package_status.py @@ -68,7 +68,7 @@ def test_malformed_visibility(): malformed_visibility = { "package_a": {"visibility": "privat3"}, "package_b": {"visibility": "publ1c"}, - "package_c": dict(), + "package_c": {}, } with pytest.raises(SystemExit) as exit_info: lint_package_status.run(malformed_visibility, REPO) diff --git a/tests/test_post_messages.py b/tests/test_post_messages.py index a661e4533..96106b53c 100644 --- a/tests/test_post_messages.py +++ b/tests/test_post_messages.py @@ -17,13 +17,13 @@ def test_get_messages(): scripts, messages, inline = get_messages_and_scripts(release, motd_db) assert "script1" in scripts assert "message1" not in messages - assert inline is not [] + assert inline != [] release = "2020.01.01-py36-rhel6" scripts, messages, inline = get_messages_and_scripts(release, motd_db) assert "script1" not in scripts assert "message1" in messages - assert inline is not [] + assert inline != [] def test_main_success(tmpdir):