Add Pylint print checker and py-checkstyle as required (#8849)

* Always run the checkstyle

* Add print checker lint plugin

* Typos

* Add print checker

* Fix e2e

.github/workflows/py-checkstyle.yml

@@ -16,9 +16,6 @@ name: Python Checkstyle
 on:
   pull_request_target:
     types: [labeled, opened, synchronize, reopened]
-    branches:
-      - main
-      - '0.[0-9]+.[0-9]+'
 
 permissions:
   contents: read

.pylintrc

@@ -19,6 +19,7 @@ good-names=T,C,fn,db
 fail-under=6.0
 init-hook='from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()))'
 extension-pkg-allow-list=pydantic
+load-plugins=ingestion.plugins.print_checker
 
 [MESSAGES CONTROL]
 disable=no-name-in-module,import-error,duplicate-code

Makefile

@@ -41,7 +41,7 @@ precommit_install:  ## Install the project's precommit hooks from .pre-commit-co
 
 .PHONY: lint
 lint: ## Run pylint on the Python sources to analyze the codebase
-	find $(PY_SOURCE) -path $(PY_SOURCE)/metadata/generated -prune -false -o -type f -name "*.py" | xargs pylint --ignore-paths=$(PY_SOURCE)/metadata_server/
+	PYTHONPATH="${PYTHONPATH}:./ingestion/plugins" find $(PY_SOURCE) -path $(PY_SOURCE)/metadata/generated -prune -false -o -type f -name "*.py" | xargs pylint --ignore-paths=$(PY_SOURCE)/metadata_server/
 
 .PHONY: py_format
 py_format:  ## Run black and isort to format the Python codebase
@@ -54,7 +54,7 @@ py_format_check:  ## Check if Python sources are correctly formatted
 	pycln ingestion/ openmetadata-airflow-apis/ --diff --extend-exclude $(PY_SOURCE)/metadata/generated
 	isort --check-only ingestion/ openmetadata-airflow-apis/ --skip $(PY_SOURCE)/metadata/generated --skip ingestion/build --profile black --multi-line 3
 	black --check --diff ingestion/ openmetadata-airflow-apis/  --extend-exclude $(PY_SOURCE)/metadata/generated
-	pylint --fail-under=10 $(PY_SOURCE)/metadata --ignore-paths $(PY_SOURCE)/metadata/generated || (echo "PyLint error code $$?"; exit 1)
+	PYTHONPATH="${PYTHONPATH}:./ingestion/plugins" pylint --fail-under=10 $(PY_SOURCE)/metadata --ignore-paths $(PY_SOURCE)/metadata/generated || (echo "PyLint error code $$?"; exit 1)
 
 ## Ingestion models generation
 .PHONY: generate

ingestion/plugins/README.md

@@ -0,0 +1,6 @@
+# Custom PyLint plugins
+
+- `pint_checker`: to handle `print` statements as warnings.
+  - Add it to `.pylintrc` as `load-plugins=ingestion.plugins.print_checker` under `[MASTER]`.
+
+You'll need to update the path of `pylint` at runtime with `PYTHONPATH="${PYTHONPATH}:./ingestion/plugins"`.

ingestion/plugins/print_checker.py

@@ -0,0 +1,53 @@
+#  Copyright 2022 Collate
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#  http://www.apache.org/licenses/LICENSE-2.0
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+"""
+Custom pylint plugin to catch `print` calls
+"""
+from typing import TYPE_CHECKING
+
+from astroid import nodes
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import only_required_for_messages
+from pylint.interfaces import IAstroidChecker
+
+if TYPE_CHECKING:
+    from pylint.lint import PyLinter
+
+
+class PrintChecker(BaseChecker):
+    """
+    Check for any print statement in the code
+    """
+
+    __implements__ = (IAstroidChecker,)
+    name = "no_print_allowed"
+    _symbol = "print-call"
+    msgs = {
+        "W5001": (
+            "Used builtin function %s",
+            _symbol,
+            "Print can make us lose traceability, use logging instead",
+        )
+    }
+
+    @only_required_for_messages("print-call")
+    def visit_call(self, node: nodes.Call) -> None:
+        if isinstance(node.func, nodes.Name) and node.func.name == "print":
+            self.add_message(self._symbol, node=node)
+
+
+def register(linter: "PyLinter") -> None:
+    """
+    This required method auto registers the checker during initialization.
+    :param linter: The linter to register the checker to.
+    """
+    linter.register_checker(PrintChecker(linter))

ingestion/src/metadata/utils/ansi.py

@@ -31,6 +31,6 @@ class ANSI(Enum):
 def print_ansi_encoded_string(
     color: Optional[ANSI] = None, bold: bool = False, message: str = ""
 ):
-    print(
+    print(  # pylint: disable=W5001
         f"{ANSI.BOLD.value if bold else ''}{color.value if color else ''}{message}{ANSI.ENDC.value}"
     )

ingestion/src/metadata/utils/time_utils.py

@@ -41,19 +41,18 @@ def get_beginning_of_day_timestamp_mill(
     """Get the beginning of day timestamp
 
     Args:
-        days (int, optional): delat in days. Defaults to 0.
-        seconds (int, optional): delat in seconds. Defaults to 0.
-        microseconds (int, optional): delat in microseconds. Defaults to 0.
-        milliseconds (int, optional): delat in milliseconds. Defaults to 0.
-        minutes (int, optional): delat in minutes. Defaults to 0.
-        hours (int, optional): delat in hours. Defaults to 0.
-        weeks (int, optional): delat in weeks. Defaults to 0.
+        days (int, optional): delay in days. Defaults to 0.
+        seconds (int, optional): delay in seconds. Defaults to 0.
+        microseconds (int, optional): delay in microseconds. Defaults to 0.
+        milliseconds (int, optional): delay in milliseconds. Defaults to 0.
+        minutes (int, optional): delay in minutes. Defaults to 0.
+        hours (int, optional): delay in hours. Defaults to 0.
+        weeks (int, optional): delay in weeks. Defaults to 0.
 
     Returns:
         int: timestamp milliseconds
     """
     now_utc = datetime.utcnow()
-    print(now_utc)
     delta = timedelta(
         weeks=weeks,
         days=days,

ingestion/tests/cli_e2e/test_cli_mysql.py

@@ -96,7 +96,7 @@ class MysqlCliTest(CliCommonDB.TestSuite):
 
     @staticmethod
     def expected_filtered_table_includes() -> int:
-        return 43
+        return 44
 
     @staticmethod
     def expected_filtered_table_excludes() -> int:

ingestion/tests/unit/data_insight/kpi/test_registry_functions.py

@@ -205,8 +205,6 @@ def test_percentage_of_entities_with_owner_kpi_result():
         kpi_target, results, "completedOwner", 1668083253659
     )
 
-    print(kpi_result)
-
     assert kpi_result.targetResult
     for result in kpi_result.targetResult:
         if result.name == "hasOwnerFraction":

ingestion/tests/unit/source/test_sagemaker.py

@@ -73,7 +73,6 @@ def _setup_mock_sagemaker(create_model: bool = False):
     sagemaker = boto3.Session().client("sagemaker")
     if not create_model:
         return
-    print("Creating model!!!!!!!")
     sagemaker.create_model(
         ModelName="mock-model",
         PrimaryContainer={