rook / rook

Storage Orchestration for Kubernetes
https://rook.io
Apache License 2.0
12k stars 2.65k forks source link

Fix suppressed pylint errors #14212

Open NymanRobin opened 2 weeks ago

NymanRobin commented 2 weeks ago

Deviation from expected behavior:

When pylint upgraded to version 3.2.0 in the CI, a new set of errors was added that failed the linting check and thus blocked CI. To unblock the CI check, a fix was added that suppressed the errors rather than fixing them in this PR #14204. However, as a follow-up to this, the error suppression on line 17 in the file deploy/examples/external/create-external-cluster-resources.py should be removed, and corresponding fixes should be made to pass the pylint check. The major problem is that two dependencies are only imported in Python 3.

if sys.version_info.major >= 3:
    py3k = True
    import urllib.parse
    from ipaddress import ip_address, IPv4Address

This then caused the following errors in pylint

deploy/examples/create-external-cluster-resources.py:592:31: E0606: Possibly using variable 'ip_address' before assignment (possibly-used-before-assignment)
deploy/examples/create-external-cluster-resources.py:592:63: E0606: Possibly using variable 'IPv4Address' before assignment (possibly-used-before-assignment)
deploy/examples/create-external-cluster-resources.py:769:19: E0606: Possibly using variable 'ip' before assignment (possibly-used-before-assignment)

The code can either be refactored to use another method to determine the ip_address and to perform an IP protocol check. However, since the usage of Python 2 is most likely minimal and not working properly at the moment, I would suggest dropping the support. This allows for a neat fix for the aforementioned pylint problem and multiple other cleanups in the code.

Expected behavior and How to reproduce it (minimal and precise):

  1. The pylint suppression # pylint: disable=E0606 should be removed from code base
  2. pylint 3.2.0 or newer should be installed
  3. command pylint $(git ls-files '*.py') -E should pass