[FEATURE] Enable pylint defaults

[dblock]

Is your feature request related to a problem?

We enabled pylint in #590. Enable the remaining lints. Here are some top ones.

$ python -m pylint setup.py noxfile.py opensearchpy/ test_opensearchpy/ utils/ samples/ benchmarks/ docs/ | grep '(' | grep '-' | rev | cut -d '(' -f1 | rev | cut -d ')' -f1 | sort | uniq -c | sort -r

1304 missing-function-docstring
 540 missing-class-docstring
 484 protected-access
 477 duplicate-code
 223 no-member
 203 missing-module-docstring
 143 unused-argument
 101 consider-using-f-string
  87 super-with-arguments
  78 too-few-public-methods
  77 redefined-builtin
  69 line-too-long
  68 too-many-arguments
  66 redefined-outer-name
  48 import-outside-toplevel
  41 useless-object-inheritance
  35 unused-variable
  32 unexpected-keyword-arg
  25 raise-missing-from
  24 too-many-locals
  23 unnecessary-dunder-call
  20 wrong-import-position
  18 too-many-public-methods
  18 invalid-unary-operand-type
  18 attribute-defined-outside-init
  17 unspecified-encoding
  16 no-else-return
  16 invalid-overridden-method
  15 cyclic-import
  11 too-many-branches
  11 dangerous-default-value
  11 arguments-renamed
  10 pointless-statement

What solution would you like?

Enable lints in setup.cfg one-by-one.

[macohen]

So, to confirm, when you say one-by-one, it's this approach:

1. add a lint
2. run pylint
3. fix ALL the failures
4. go to 1 until out of lints or I just can't anymore. ;)

[macohen]

Doing this incrementally, what do you think of breaking this down into an issue per lint? It becomes a much less daunting single task and can be picked up in parallel.

[dblock]

@macohen Yes, I don't expect all the lints to be enabled at once, that's a massive change.

[macohen]

Running utils/build_dists.py results in an error:

Installing collected packages: urllib3, six, idna, charset-normalizer, certifi, requests, python-dateutil, opensearch-py2
Successfully installed certifi-2023.11.17 charset-normalizer-3.3.2 idna-3.6 opensearch-py2-2.4.3 python-dateutil-2.8.2 requests-2.31.0 six-1.16.0 urllib3-1.26.18
$ /var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/bin/python -c 'from opensearchpy2 import OpenSearch, Q'
Traceback (most recent call last):
File "", line 1, in
File "/private/var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/lib/python3.10/site-packages/opensearchpy2/init.py", line 46, in
from .client import OpenSearch
File "/private/var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/lib/python3.10/site-packages/opensearchpy2/client/init.py", line 42, in
from ..transport import Transport, TransportError
File "/private/var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/lib/python3.10/site-packages/opensearchpy2/transport.py", line 40, in
from .serializer import DEFAULT_SERIALIZERS, Deserializer, JSONSerializer, Serializer
File "/private/var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/lib/python3.10/site-packages/opensearchpy2/serializer.py", line 41, in
from .helpers.utils import AttrList
File "/private/var/folders/33/jx0mw87156q2hmtrr_r82s7r0000gs/T/tmpko79lorv/venv/lib/python3.10/site-packages/opensearchpy2/helpers/utils.py", line 36, in
from opensearchpy.exceptions import UnknownDslObject, ValidationException
ModuleNotFoundError: No module named 'opensearchpy'
Command exited incorrectly: should have been 0 was 256

Before I go down a rabbit hole, I think there may be some missing instruction about how to use this or it may not work correctly. I see a docstring:

A command line tool for building and verifying releases
Can be used for building both 'opensearchpy' and 'opensearchpyX' dists.
Only requires 'name' in 'setup.py' and the directory to be changed.

What's the intent of an opensearchpyX distribution? What name and directory should be changed?

[dblock]

@macohen I don't know anything about this one, I would start by checking whether this is dead code/used in any of our release GHA versions.

[macohen]

got that one. it is only called from GitHub Actions. Also, I've been adding a lot of "# pylint: disable=missing-function-docstring" instructions to any function starting with def test_ or async def test_. All of those functions have pretty good descriptions as function names so I thought this would be a good exception. Even wrote a script for it that I'm adding to utils. Could use some improvements on multi-line function definitions...

I was thinking about adding to ignore in pylint in setup.cfg for the test_opensearchpy directory, but that seems too heavy handed...

[macohen]

Should the opensearchpy directory be ignored for pylint? That code is generated (I think) so maybe the generator needs to include the docstrings and other items.

[dblock]

I don't think tests need descriptions, so I would try to ignore test folders for that linter specifically, but not others. The generator definitely should include docstrings.

[macohen]

Tracking them here as I add:

• missing-function-docstring
• missing-class-docstring
• protected-access - tried this one locally; need more in-depth understanding of the code.
• duplicate-code
• no-member
• missing-module-docstring
• unused-argument
• consider-using-f-string
• super-with-arguments
• too-few-public-methods
• redefined-builtin
• line-too-long
• too-many-arguments
• redefined-outer-name
• import-outside-toplevel
• useless-object-inheritance
• unused-variable Assignment from no return #658
• unexpected-keyword-arg
• raise-missing-from
• too-many-locals
• unnecessary-dunder-call (PR out on 2024-01-21)
• assignment-from-no-return (PR Assignment from no return #658)
• wrong-import-position
• too-many-public-methods
• invalid-unary-operand-type
• attribute-defined-outside-init
• unspecified-encoding
• no-else-return
• invalid-overridden-method
• cyclic-import
• too-many-branches
• dangerous-default-value
• arguments-renamed
• pointless-statement

[macohen]

@saimedhi is there a priority on the lints you'd like to see above? I've been tackling them based on what looks interesting to report on and not too much of a rabbit hole. I'll start updating my findings in the comment above as well.

[saimedhi]

Feel free to address the lints as you see fit; I don't have any specific priorities.