Pylint integration updates

CHANGELOG.md

@@ -3,6 +3,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 ### Added
+- Added pylint `unspecified-encoding` and `missing-function-docstring` and ignored opensearchpy for lints; added new linting per directory option behind feature flag (([#643](https://github.com/opensearch-project/opensearch-py/pull/643)))
 - Added pylint `line-too-long` and `invalid-name` ([#590](https://github.com/opensearch-project/opensearch-py/pull/590))
 - Added pylint `pointless-statement` ([#611](https://github.com/opensearch-project/opensearch-py/pull/611))
 - Added a log collection guide ([#579](https://github.com/opensearch-project/opensearch-py/pull/579))

noxfile.py

@@ -43,6 +43,7 @@
 
 @nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10", "3.11"])  # type: ignore
 def test(session: Any) -> None:
+    # pylint: disable=missing-function-docstring
     session.install(".")
     # ensure client can be imported without aiohttp
     session.run("python", "-c", "import opensearchpy\nprint(opensearchpy.OpenSearch())")
@@ -59,6 +60,7 @@ def test(session: Any) -> None:
 
 @nox.session(python=["3.7"])  # type: ignore
 def format(session: Any) -> None:
+    # pylint: disable=missing-function-docstring
     session.install(".")
     session.install("black", "isort")
 
@@ -71,6 +73,7 @@ def format(session: Any) -> None:
 
 @nox.session(python=["3.7"])  # type: ignore
 def lint(session: Any) -> None:
+    # pylint: disable=missing-function-docstring
     session.install(
         "flake8",
         "black",
@@ -89,7 +92,15 @@ def lint(session: Any) -> None:
     session.run("isort", "--check", *SOURCE_FILES)
     session.run("black", "--check", *SOURCE_FILES)
     session.run("flake8", *SOURCE_FILES)
-    session.run("pylint", *SOURCE_FILES)
+    if (
+        # run export NOXFILE_PYLINT_PARAMS_FEATURE=true on the command line to run this code
+        "NOXFILE_PYLINT_PARAMS_FEATURE" in session.env
+        and session.env["NOXFILE_PYLINT_PARAMS_FEATURE"]
+    ):
+        lint_per_folder(session)
+    else:
+        session.run("pylint", *SOURCE_FILES)
+
     session.run("python", "utils/license_headers.py", "check", *SOURCE_FILES)
 
     # Workaround to make '-r' to still work despite uninstalling aiohttp below.
@@ -108,8 +119,60 @@ def lint(session: Any) -> None:
     session.run("mypy", "--strict", "test_opensearchpy/test_types/sync_types.py")
 
 
+def lint_per_folder(session: Any) -> None:
+    """
+    allows configuration of pylint rules per folder and runs a pylint command for each folder
+    :param session: the current nox session
+    """
+    # tests should not require function docstrings - tests function names describe themselves;
+    # opensearchpy is generated; may require in the generator code some places
+    default_enable = [
+        "line-too-long",
+        "invalid-name",
+        "pointless-statement",
+        "unspecified-encoding",
+        "missing-function-docstring",
+    ]
+    override_enable = {
+        "test_opensearchpy/": [
+            "line-too-long",
+            # "invalid-name", lots of short functions with one or two character names
+            "pointless-statement",
+            "unspecified-encoding",
+            "redefined-outer-name",
+        ],
+        # "opensearchpy/": [""],
+    }
+    # import-outside-toplevel
+    # enable = line-too-long, invalid-name, pointless-statement, unspecified-encoding,
+    # missing-function-docstring
+    # should fail the build: redefined-outer-name, , line-too-long, invalid-name,
+    # pointless-statement,
+    # import-outside-toplevel, unused-variable, unexpected-keyword-arg,
+    # raise-missing-from, invalid-unary-operand-type,
+    # attribute-defined-outside-init, unspecified-encoding
+    # should be warnings: super-with-arguments, too-few-public-methods, redefined-builtin,
+    # too-many-arguments
+    # (how many is too many?), useless-object-inheritance, too-many-locals,
+    # too-many-branches, dangerous-default-value,
+    # arguments-renamed
+    # warn, then fail later (low priority): too-many-locals, unnecessary-dunder-call,
+    # too-many-public-methods,
+    # no-else-return, invalid-overridden-method, cyclic-import
+    # does this conflict with isort? wrong-import-position
+    for source_file in SOURCE_FILES:
+        args = ["--disable=all"]
+        if source_file in override_enable:
+            args.append(f"--enable={','.join(override_enable[source_file])}")
+        else:
+            args.append(f"--enable={','.join(default_enable)}")
+        args.append(source_file)
+        session.run("pylint", *args)
+
+
 @nox.session()  # type: ignore
 def docs(session: Any) -> None:
+    # pylint: disable=missing-function-docstring
     session.install(".")
     session.install(".[docs]")
     with session.chdir("docs"):
@@ -118,6 +181,7 @@ def docs(session: Any) -> None:
 
 @nox.session()  # type: ignore
 def generate(session: Any) -> None:
+    # pylint: disable=missing-function-docstring
     session.install("-rdev-requirements.txt")
     session.run("python", "utils/generate_api.py")
     format(session)

setup.cfg

@@ -28,4 +28,5 @@ good-names-rgxs = ^[_a-z][_a-z0-9]?$  # allow for 1-character variable names
 
 [pylint.MESSAGE CONTROL]
 disable = all
-enable = line-too-long, invalid-name, pointless-statement
+enable = line-too-long, invalid-name, pointless-statement, missing-function-docstring, unspecified-encoding
+ignore=opensearchpy

setup.py

@@ -34,15 +34,17 @@
 PACKAGE_VERSION = ""
 BASE_DIR = abspath(dirname(__file__))
 
-with open(join(BASE_DIR, PACKAGE_NAME.replace("-", ""), "_version.py")) as f:
+with open(
+    join(BASE_DIR, PACKAGE_NAME.replace("-", ""), "_version.py"), encoding="utf-8"
+) as f:
     data = f.read()
     m = re.search(r"^__versionstr__: str\s+=\s+[\"\']([^\"\']+)[\"\']", data, re.M)
     if m:
         PACKAGE_VERSION = m.group(1)
     else:
         raise Exception(f"Invalid version: {data}")
 
-with open(join(BASE_DIR, "README.md")) as f:
+with open(join(BASE_DIR, "README.md"), encoding="utf-8") as f:
     long_description = f.read().strip()
 
 MODULE_DIR = PACKAGE_NAME.replace("-", "")

test_opensearchpy/run_tests.py

@@ -41,6 +41,11 @@
 
 
 def fetch_opensearch_repo() -> None:
+    """
+    runs a git fetch origin on configured opensearch core repo
+    :return: None if environmental variables TEST_OPENSEARCH_YAML_DIR is set or TEST_OPENSEARCH_NOFETCH is set to False;
+    else returns nothing
+    """
     # user is manually setting YAML dir, don't tamper with it
     if "TEST_OPENSEARCH_YAML_DIR" in environ:
         return
@@ -90,6 +95,16 @@ def fetch_opensearch_repo() -> None:
 
 
 def run_all(argv: Any = None) -> None:
+    """
+    run all the tests given arguments and environment variables
+    - sets defaults if argv is None, running "pytest --cov=opensearchpy --junitxml=<path to opensearch-py-junit.xml>
+    --log-level=DEBUG --cache-clear -vv --cov-report=<path to output code coverage"
+    * GITHUB_ACTION: fetches yaml tests if this is not in environment variables
+    * TEST_PATTERN: specify a test to run
+    * TEST_TYPE: "server" runs on TLS connection; None is unencrypted
+    * OPENSEARCH_VERSION: "SNAPSHOT" does not do anything with plugins
+    :param argv: if this is None, then the default arguments
+    """
     sys.exitfunc = lambda: sys.stderr.write("Shutting down....\n")  # type: ignore
     # fetch yaml tests anywhere that's not GitHub Actions
     if "GITHUB_ACTION" not in environ:

test_opensearchpy/test_async/test_client.py

@@ -63,28 +63,34 @@ async def perform_request(
 
 class OpenSearchTestCaseWithDummyTransport:
     def assert_call_count_equals(self, count: int) -> None:
+        # pylint: disable=missing-function-docstring
         assert isinstance(self.client.transport, DummyTransport)
         assert count == self.client.transport.call_count
 
     def assert_url_called(self, method: str, url: str, count: int = 1) -> Any:
+        # pylint: disable=missing-function-docstring
         assert isinstance(self.client.transport, DummyTransport)
         assert (method, url) in self.client.transport.calls
         calls = self.client.transport.calls[(method, url)]
         assert count == len(calls)
         return calls
 
     def setup_method(self, method: Any) -> None:
+        # pylint: disable=missing-function-docstring
         self.client = AsyncOpenSearch(transport_class=DummyTransport)
 
 
 class TestClient(OpenSearchTestCaseWithDummyTransport):
     async def test_our_transport_used(self) -> None:
+        # pylint: disable=missing-function-docstring
         assert isinstance(self.client.transport, DummyTransport)
 
     async def test_start_with_0_call(self) -> None:
+        # pylint: disable=missing-function-docstring
         self.assert_call_count_equals(0)
 
     async def test_each_call_is_recorded(self) -> None:
+        # pylint: disable=missing-function-docstring
         await self.client.transport.perform_request("GET", "/")
         await self.client.transport.perform_request(
             "DELETE", "/42", params={}, body="body"