From 6a36086736970a122513c3473174557a8c5dec49 Mon Sep 17 00:00:00 2001 From: Topher Anderson <48180628+topherinternational@users.noreply.github.com> Date: Thu, 7 Dec 2023 22:24:36 +0100 Subject: [PATCH] require pylint 2.14.0+ (incl. 3.x.x), Python 3.7/3.8 (#3) --- .pylintrc | 85 +------------------ Makefile | 8 +- docker/ci.Dockerfile | 6 +- scripts/ci_validate_msg_ids.py | 10 +-- scripts/generate_codes_table.py | 9 +- setup.py | 12 +-- src/pylint_airflow/checkers/dag.py | 5 +- src/pylint_airflow/checkers/operator.py | 9 +- src/pylint_airflow/checkers/xcom.py | 11 +-- tests/conftest.py | 24 ++---- tests/pylint_airflow/checkers/test_dag.py | 5 +- .../pylint_airflow/checkers/test_operator.py | 18 ++-- tests/pylint_airflow/checkers/test_xcom.py | 5 +- 13 files changed, 61 insertions(+), 146 deletions(-) diff --git a/.pylintrc b/.pylintrc index 1fc8379..305f984 100644 --- a/.pylintrc +++ b/.pylintrc @@ -60,17 +60,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use "--disable=all --enable=classes # --disable=W". -disable=print-statement, - parameter-unpacking, - unpacking-in-except, - old-raise-syntax, - backtick, - long-suffix, - old-ne-operator, - old-octal-literal, - import-star-module-level, - non-ascii-bytes-literal, - raw-checker-failed, +disable=raw-checker-failed, bad-inline-option, locally-disabled, file-ignored, @@ -78,68 +68,6 @@ disable=print-statement, useless-suppression, deprecated-pragma, use-symbolic-message-instead, - apply-builtin, - basestring-builtin, - buffer-builtin, - cmp-builtin, - coerce-builtin, - execfile-builtin, - file-builtin, - long-builtin, - raw_input-builtin, - reduce-builtin, - standarderror-builtin, - unicode-builtin, - xrange-builtin, - coerce-method, - delslice-method, - getslice-method, - setslice-method, - no-absolute-import, - old-division, - dict-iter-method, - dict-view-method, - next-method-called, - metaclass-assignment, - indexing-exception, - raising-string, - reload-builtin, - oct-method, - hex-method, - nonzero-method, - cmp-method, - input-builtin, - round-builtin, - intern-builtin, - unichr-builtin, - map-builtin-not-iterating, - zip-builtin-not-iterating, - range-builtin-not-iterating, - filter-builtin-not-iterating, - using-cmp-argument, - eq-without-hash, - div-method, - idiv-method, - rdiv-method, - exception-message-attribute, - invalid-str-codec, - sys-max-int, - bad-python3-import, - deprecated-string-function, - deprecated-str-translate-call, - deprecated-itertools-function, - deprecated-types-field, - next-method-defined, - dict-items-not-iterating, - dict-keys-not-iterating, - dict-values-not-iterating, - deprecated-operator-function, - deprecated-urllib-function, - xreadlines-attribute, - deprecated-sys-function, - exception-escape, - comprehension-escape, - bad-continuation, # https://github.com/PyCQA/pylint/issues/289 duplicate-code, fixme @@ -329,13 +257,6 @@ max-line-length=100 # Maximum number of lines in a module. max-module-lines=1000 -# List of optional constructs for which whitespace checking is disabled. `dict- -# separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. -# `trailing-comma` allows a space between comma and closing bracket: (a, ). -# `empty-line` allows space-only lines. -no-space-check=trailing-comma, - dict-separator - # Allow the body of a class to be on the same line as the declaration if body # contains single statement. single-line-class-stmt=no @@ -569,5 +490,5 @@ min-public-methods=2 # Exceptions that will emit a warning when being caught. Defaults to # "BaseException, Exception". -overgeneral-exceptions=BaseException, - Exception +overgeneral-exceptions=builtins.BaseException, + builtins.Exception diff --git a/Makefile b/Makefile index 3e16bf0..373229b 100644 --- a/Makefile +++ b/Makefile @@ -24,13 +24,13 @@ ci: | clean-compiled black pylint validate_message_ids pytest .PHONY: ci-docker ci-docker: build_ci_image - docker run -ti -v `pwd`:/pylint-airflow -w /pylint-airflow basph/pylint-airflow-ci:3.6-slim bash -c "pip install .; make ci" - docker run -ti -v `pwd`:/pylint-airflow -w /pylint-airflow basph/pylint-airflow-ci:3.7-slim bash -c "pip install .; make ci" + docker run -ti -v `pwd`:/pylint-airflow -w /pylint-airflow pylint-airflow/pylint-airflow-ci:3.7-slim bash -c "pip install .; make ci" + docker run -ti -v `pwd`:/pylint-airflow -w /pylint-airflow pylint-airflow/pylint-airflow-ci:3.8-slim bash -c "pip install .; make ci" .PHONY: build_ci_image build_ci_image: - docker build --file docker/ci.Dockerfile --progress plain --build-arg PYTHON_VERSION=3.6-slim --tag basph/pylint-airflow-ci:3.6-slim . - docker build --file docker/ci.Dockerfile --progress plain --build-arg PYTHON_VERSION=3.7-slim --tag basph/pylint-airflow-ci:3.7-slim . + docker build --file docker/ci.Dockerfile --build-arg PYTHON_VERSION=3.7-slim --tag pylint-airflow/pylint-airflow-ci:3.7-slim . + docker build --file docker/ci.Dockerfile --build-arg PYTHON_VERSION=3.8-slim --tag pylint-airflow/pylint-airflow-ci:3.8-slim . .PHONY: upload-to-pypi upload-to-pypi: diff --git a/docker/ci.Dockerfile b/docker/ci.Dockerfile index 2622b4e..fdbd67e 100644 --- a/docker/ci.Dockerfile +++ b/docker/ci.Dockerfile @@ -10,9 +10,9 @@ COPY requirements.txt requirements.txt # https://github.com/apache/airflow/pull/4513 RUN apt-get update && \ apt-get install -y gcc g++ make --no-install-recommends && \ - pip install -U pip && \ - SLUGIFY_USES_TEXT_UNIDECODE=yes pip install -r requirements.txt && \ - apt-get remove -y --purge gcc g++ && \ + pip install --upgrade pip +RUN SLUGIFY_USES_TEXT_UNIDECODE=yes pip install -r requirements.txt +RUN apt-get remove -y --purge gcc g++ && \ apt-get autoremove -y && \ apt-get clean -y && \ rm -rf /var/lib/apt/lists/* diff --git a/scripts/ci_validate_msg_ids.py b/scripts/ci_validate_msg_ids.py index 5f4cc2d..750b4ed 100644 --- a/scripts/ci_validate_msg_ids.py +++ b/scripts/ci_validate_msg_ids.py @@ -4,9 +4,9 @@ will be interpreted incorrectly). 2. For each message type, check if codes start at 0 and increment by 1, e.g. C8300, C8301, ... """ -import os import re from collections import defaultdict +from pathlib import Path from typing import List from pylint.lint import PyLinter @@ -45,11 +45,11 @@ def check_if_msg_ids_in_readme(message_ids: List[str]): :param List[str] message_ids: All message IDs found in pylint-airflow """ - readme_path = os.path.join(os.path.abspath(os.path.dirname(__file__)), "../README.rst") - with open(readme_path, "r") as readme_: - readme = readme_.read() + readme_path = Path(__file__).resolve().parent.parent / "README.rst" + with open(readme_path, mode="r", encoding="utf-8") as readme_file: + readme_text = readme_file.read() - not_found = [msg_id for msg_id in message_ids if msg_id not in readme] + not_found = [msg_id for msg_id in message_ids if msg_id not in readme_text] if not_found: raise AssertionError( f"Message IDs {not_found} not found in README. All message IDs should be documented." diff --git a/scripts/generate_codes_table.py b/scripts/generate_codes_table.py index 4846a9d..bc549a4 100644 --- a/scripts/generate_codes_table.py +++ b/scripts/generate_codes_table.py @@ -126,9 +126,8 @@ def gen_content(msgs: Dict[str, Dict[str, Tuple[str, str]]], lengths: List[int]) ] result = "\n".join(table) -print( - "{color_red}Copy the following into README.rst:{no_color}\n".format( - color_red="\x1b[1;31;40m", no_color="\x1b[0m" - ) -) +color_red = "\x1b[1;31;40m" +no_color = "\x1b[0m" + +print(f"{color_red}Copy the following into README.rst:{no_color}\n") print(result) diff --git a/setup.py b/setup.py index 9850b3d..2ef1bbd 100644 --- a/setup.py +++ b/setup.py @@ -3,11 +3,11 @@ from setuptools import setup, find_packages -requirements = ["pylint>=2.5.0,<2.7.0"] +requirements = ["pylint>=2.14.0"] -readme = Path(__file__).resolve().parent / "README.rst" -with open(readme) as f: - long_description = f.read() +readme_path = Path(__file__).resolve().parent / "README.rst" +with open(readme_path, encoding="utf-8") as readme_file: + long_description = readme_file.read() setup( name="pylint-airflow", @@ -15,10 +15,11 @@ description="A Pylint plugin to lint Apache Airflow code.", long_description=long_description, long_description_content_type="text/x-rst", - version="0.1.1-alpha.1", + version="0.2.1", packages=find_packages(where="src"), package_dir={"": "src"}, install_requires=requirements, + python_requires=">=3.7", keywords=["pylint", "airflow", "plugin"], classifiers=[ "Development Status :: 3 - Alpha", @@ -26,7 +27,6 @@ "License :: OSI Approved :: MIT License", "Natural Language :: English", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", ], diff --git a/src/pylint_airflow/checkers/dag.py b/src/pylint_airflow/checkers/dag.py index aefb51f..87b7eda 100644 --- a/src/pylint_airflow/checkers/dag.py +++ b/src/pylint_airflow/checkers/dag.py @@ -5,7 +5,6 @@ import astroid from pylint import checkers -from pylint import interfaces from pylint.checkers import utils from pylint.checkers.utils import safe_infer @@ -15,8 +14,6 @@ class DagChecker(checkers.BaseChecker): """Checks conditions in the context of (a) complete DAG(s).""" - __implements__ = interfaces.IAstroidChecker - msgs = { f"W{BASE_ID}00": ( "Don't place BaseHook calls at the top level of DAG script", @@ -57,7 +54,7 @@ class DagChecker(checkers.BaseChecker): ), } - @utils.check_messages("duplicate-dag-name", "match-dagid-filename") + @utils.only_required_for_messages("duplicate-dag-name", "match-dagid-filename") def visit_module(self, node: astroid.Module): """Checks in the context of (a) complete DAG(s).""" dagids_nodes = defaultdict(list) diff --git a/src/pylint_airflow/checkers/operator.py b/src/pylint_airflow/checkers/operator.py index 83f0b31..e297dba 100644 --- a/src/pylint_airflow/checkers/operator.py +++ b/src/pylint_airflow/checkers/operator.py @@ -2,7 +2,6 @@ import astroid from pylint import checkers -from pylint import interfaces from pylint.checkers import utils from pylint.checkers.utils import safe_infer @@ -12,8 +11,6 @@ class OperatorChecker(checkers.BaseChecker): """Checks on Airflow operators.""" - __implements__ = (interfaces.IAstroidChecker,) - msgs = { f"C{BASE_ID}00": ( "Operator variable name and task_id argument should match", @@ -52,7 +49,7 @@ class OperatorChecker(checkers.BaseChecker): ), } - @utils.check_messages("different-operator-varname-taskid", "match-callable-taskid") + @utils.only_required_for_messages("different-operator-varname-taskid", "match-callable-taskid") def visit_assign(self, node): """ TODO rewrite this @@ -95,7 +92,7 @@ def invalidname(): print("dosomething") if python_callable_name and f"_{task_id}" != python_callable_name: self.add_message("match-callable-taskid", node=node) - @utils.check_messages("mixed-dependency-directions") + @utils.only_required_for_messages("mixed-dependency-directions") def visit_binop(self, node): """Check for mixed dependency directions.""" @@ -109,7 +106,7 @@ def fetch_binops(node_): binops_found.update(fetch_binops(node_.left)) if isinstance(node_.right, astroid.BinOp): binops_found.update(fetch_binops(node_.right)) - if node_.op == ">>" or node_.op == "<<": + if node_.op in (">>", "<<"): binops_found.add(node_.op) return binops_found diff --git a/src/pylint_airflow/checkers/xcom.py b/src/pylint_airflow/checkers/xcom.py index 5e03414..e649224 100644 --- a/src/pylint_airflow/checkers/xcom.py +++ b/src/pylint_airflow/checkers/xcom.py @@ -2,7 +2,6 @@ import astroid from pylint import checkers -from pylint import interfaces from pylint.checkers import utils from pylint_airflow.__pkginfo__ import BASE_ID @@ -11,8 +10,6 @@ class XComChecker(checkers.BaseChecker): """Checks on Airflow XComs.""" - __implements__ = interfaces.IAstroidChecker - msgs = { f"R{BASE_ID}00": ( "Return value from %s is stored as XCom but not used anywhere", @@ -22,7 +19,7 @@ class XComChecker(checkers.BaseChecker): ) } - @utils.check_messages("unused-xcom") + @utils.only_required_for_messages("unused-xcom") def visit_module(self, node: astroid.Module): """ Check for unused XComs. @@ -37,7 +34,7 @@ def visit_module(self, node: astroid.Module): # Store nodes containing python_callable arg as: # {task_id: (call node, python_callable func name)} - python_callable_nodes = dict() + python_callable_nodes = {} for call_node in call_nodes: if call_node.keywords: task_id = "" @@ -53,7 +50,7 @@ def visit_module(self, node: astroid.Module): python_callable_nodes[task_id] = (call_node, python_callable) # Now fetch the functions mentioned by python_callable args - xcoms_pushed = dict() + xcoms_pushed = {} xcoms_pulled_taskids = set() for (task_id, (python_callable, callable_func_name)) in python_callable_nodes.items(): if callable_func_name != "": @@ -65,7 +62,7 @@ def visit_module(self, node: astroid.Module): callable_func = node.getattr(callable_func_name)[0] # Check if the function returns any values - if any([isinstance(n, astroid.Return) for n in callable_func.body]): + if any(isinstance(n, astroid.Return) for n in callable_func.body): # Found a return statement xcoms_pushed[task_id] = (python_callable, callable_func_name) diff --git a/tests/conftest.py b/tests/conftest.py index ff4bbe8..052758e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,14 +1,11 @@ """Globally accessible helpers functions/classes in all tests.""" -import io import os import pytest from pylint.testutils import ( LintModuleTest, FunctionalTestFile, - multiset_difference, - get_expected_messages, ) pytest_plugins = ["helpers_namespace"] @@ -40,31 +37,28 @@ def __init__(self, test_filepath): self._test_filepath = test_filepath self._linter.load_plugin_modules(["pylint_airflow"]) - def _get_expected(self): - with io.open(self._test_filepath, encoding="utf8") as fobj: - return get_expected_messages(fobj) + def _get_expected_messages(self): + with self._open_source_file() as test_file: + return self.get_expected_messages(test_file) def check_file(self): """Run Pylint on a file.""" - self._linter.check(self._test_filepath) + self._linter.check((self._test_filepath,)) - expected_msgs = self._get_expected() - received_msgs, received_text = self._get_received() + expected_msgs = self._get_expected_messages() + received_msgs, received_text = self._get_actual() linesymbol_text = {(ol.lineno, ol.symbol): ol.msg for ol in received_text} if expected_msgs != received_msgs: - missing, unexpected = multiset_difference(expected_msgs, received_msgs) + missing, unexpected = self.multiset_difference(expected_msgs, received_msgs) msg = [f"Wrong results for file '{self._test_file.base}':"] if missing: msg.append("\nExpected in testdata:") - msg.extend( - f" {line_nr:3d}: {symbol} - {linesymbol_text[(line_nr, symbol)]}" - for line_nr, symbol in sorted(missing) - ) + msg.extend(symbol for _, symbol in sorted(missing)) if unexpected: msg.append("\nUnexpected in testdata:") msg.extend( - f" {line_nr:3d}: {symbol} - {linesymbol_text[(line_nr,symbol)]}" + f" {line_nr:3d}: {symbol} - {linesymbol_text[(line_nr, symbol)]}" for line_nr, symbol in sorted(unexpected) ) pytest.fail("\n".join(msg)) diff --git a/tests/pylint_airflow/checkers/test_dag.py b/tests/pylint_airflow/checkers/test_dag.py index e3ce776..262fc1d 100644 --- a/tests/pylint_airflow/checkers/test_dag.py +++ b/tests/pylint_airflow/checkers/test_dag.py @@ -1,7 +1,7 @@ """Tests for the DAG checker.""" import astroid -from pylint.testutils import CheckerTestCase, Message +from pylint.testutils import CheckerTestCase, MessageTest import pylint_airflow @@ -27,7 +27,8 @@ def test_duplicate_dag(self): ast = astroid.parse(testcase) expected_msg_node = ast.body[4].value with self.assertAddsMessages( - Message(msg_id="duplicate-dag-name", node=expected_msg_node, args="lintme") + MessageTest(msg_id="duplicate-dag-name", node=expected_msg_node, args="lintme"), + ignore_position=True, ): self.checker.visit_module(ast) diff --git a/tests/pylint_airflow/checkers/test_operator.py b/tests/pylint_airflow/checkers/test_operator.py index b0055c9..f29d6b5 100644 --- a/tests/pylint_airflow/checkers/test_operator.py +++ b/tests/pylint_airflow/checkers/test_operator.py @@ -2,7 +2,7 @@ import astroid import pytest -from pylint.testutils import CheckerTestCase, Message +from pylint.testutils import CheckerTestCase, MessageTest import pylint_airflow @@ -21,7 +21,9 @@ def test_different_operator_varname_taskid(self): expected_message = "different-operator-varname-taskid" assign_node = astroid.extract_node(testcase) - with self.assertAddsMessages(Message(msg_id=expected_message, node=assign_node)): + with self.assertAddsMessages( + MessageTest(msg_id=expected_message, node=assign_node), ignore_position=True + ): self.checker.visit_assign(assign_node) def test_different_operator_varname_taskid_baseoperator(self): @@ -36,7 +38,9 @@ def test_different_operator_varname_taskid_baseoperator(self): expected_message = "different-operator-varname-taskid" assign_node = astroid.extract_node(testcase) - with self.assertAddsMessages(Message(msg_id=expected_message, node=assign_node)): + with self.assertAddsMessages( + MessageTest(msg_id=expected_message, node=assign_node), ignore_position=True + ): self.checker.visit_assign(assign_node) def test_different_operator_varname_taskid_valid(self): @@ -73,7 +77,9 @@ def test_match_callable_taskid(self, imports, operator_def): expected_message = "match-callable-taskid" assign_node = astroid.extract_node(testcase) - with self.assertAddsMessages(Message(msg_id=expected_message, node=assign_node)): + with self.assertAddsMessages( + MessageTest(msg_id=expected_message, node=assign_node), ignore_position=True + ): self.checker.visit_assign(assign_node) def test_not_match_callable_taskid(self): @@ -123,7 +129,9 @@ def test_mixed_dependency_directions(self, dependencies, expect_msg): binop_node = astroid.extract_node(testcase) if expect_msg: - with self.assertAddsMessages(Message(msg_id=message, node=binop_node)): + with self.assertAddsMessages( + MessageTest(msg_id=message, node=binop_node), ignore_position=True + ): self.checker.visit_binop(binop_node) else: with self.assertNoMessages(): diff --git a/tests/pylint_airflow/checkers/test_xcom.py b/tests/pylint_airflow/checkers/test_xcom.py index c686a01..e458580 100644 --- a/tests/pylint_airflow/checkers/test_xcom.py +++ b/tests/pylint_airflow/checkers/test_xcom.py @@ -1,7 +1,7 @@ """Tests for the XCom checker.""" import astroid -from pylint.testutils import CheckerTestCase, Message +from pylint.testutils import CheckerTestCase, MessageTest import pylint_airflow @@ -51,6 +51,7 @@ def _pulltask(): expected_msg_node = ast.body[2].value expected_args = "_pushtask" with self.assertAddsMessages( - Message(msg_id="unused-xcom", node=expected_msg_node, args=expected_args) + MessageTest(msg_id="unused-xcom", node=expected_msg_node, args=expected_args), + ignore_position=True, ): self.checker.visit_module(ast)