pylint: fix W0613,W0621,W1406 (#1282)

pyproject.toml

@@ -32,6 +32,9 @@ disable = [
   "R0913",  # too-many-arguments
   "R0914",  # too-many-locals
   "R0915",  # too-many-statements
+  "W0511",  # fixme
+  "W0718",  # broad-exception-caught
+  "W0719",  # broad-exception-raised
 ]
 
 enable = [
@@ -58,7 +61,10 @@ enable = [
   "W0201",  # attribute-defined-outside-init
   "W0212",  # protected-access
   "W0612",  # unused-variable
+  "W0613",  # unused-argument
+  "W0621",  # redefined-outer-name
   "W0707",  # raise-missing-from
   "W1202",  # logging-format-interpolation
   "W1203",  # logging-fstring-interpolation
+  "W1406",  # redundant-u-string-prefix
 ]

src/ansible_runner/config/_base.py

@@ -66,6 +66,8 @@ def __init__(self,
                  container_image=None, container_volume_mounts=None, container_options=None, container_workdir=None, container_auth_data=None,
                  ident=None, rotate_artifacts=0, timeout=None, ssh_key=None, quiet=False, json_mode=False,
                  check_job_event_data=False, suppress_env_files=False, keepalive_seconds=None):
+        # pylint: disable=W0613
+
         # common params
         self.host_cwd = host_cwd
         self.envvars = envvars

src/ansible_runner/runner.py

@@ -336,10 +336,10 @@ def run(self):
                     #     extra_update_fields['job_explanation'] = "Job terminated due to timeout"
                 if self.canceled or self.timed_out or self.errored:
                     self.kill_container()
-                    Runner.handle_termination(child.pid, is_cancel=self.canceled)
+                    Runner.handle_termination(child.pid)
                 if self.config.idle_timeout and (time.time() - self.last_stdout_update) > self.config.idle_timeout:
                     self.kill_container()
-                    Runner.handle_termination(child.pid, is_cancel=False)
+                    Runner.handle_termination(child.pid)
                     self.timed_out = True
 
             stdout_handle.close()
@@ -526,14 +526,12 @@ def kill_container(self):
                     logger.info("Killed container %s", container_name)
 
     @classmethod
-    def handle_termination(cls, pid, pidfile=None, is_cancel=True):
+    def handle_termination(cls, pid, pidfile=None):
         '''
         Internal method to terminate a subprocess spawned by ``pexpect`` representing an invocation of runner.
 
         :param pid:       the process id of the running the job.
         :param pidfile:   the daemon's PID file
-        :param is_cancel: flag showing whether this termination is caused by
-                          instance's cancel_flag.
         '''
 
         try:

src/ansible_runner/streaming.py

@@ -223,6 +223,7 @@ def run(self):
 
     @_synchronize_output_reset_keepalive  # type: ignore
     def status_handler(self, status_data, runner_config):
+        # pylint: disable=W0613
         self.status = status_data['status']
         self._output.write(json.dumps(status_data).encode('utf-8'))
         self._output.write(b'\n')
@@ -241,6 +242,7 @@ def artifacts_handler(self, artifact_dir):
 
     @_synchronize_output_reset_keepalive  # type: ignore
     def finished_callback(self, runner_obj):
+        # pylint: disable=W0613
         self._end_keepalive()  # ensure that we can't splat a keepalive event after the eof event
         self._output.write(json.dumps({'eof': True}).encode('utf-8'))
         self._output.write(b'\n')
@@ -304,15 +306,15 @@ def status_callback(self, status_data):
     def event_callback(self, event_data):
         # FIXME: this needs to be more defensive to not blow up on "malformed" events or new values it doesn't recognize
         counter = event_data.get('counter')
-        uuid = event_data.get('uuid')
+        uuid_val = event_data.get('uuid')
 
-        if not counter or not uuid:
+        if not counter or not uuid_val:
             # FIXME: log a warning about a malformed event?
             return
 
         full_filename = os.path.join(self.artifact_dir,
                                      'job_events',
-                                     f'{counter}-{uuid}.json')
+                                     f'{counter}-{uuid_val}.json')
 
         if not self.quiet and 'stdout' in event_data:
             print(event_data['stdout'])

src/ansible_runner/utils/__init__.py

@@ -500,6 +500,7 @@ def signal_handler():
 
     # closure to set signal event
     def _handler(number, frame):
+        # pylint: disable=W0613
         signal_event.set()
 
     signal.signal(signal.SIGTERM, _handler)

test/conftest.py

@@ -1,3 +1,5 @@
+# pylint: disable=W0621
+
 import shutil
 
 from pathlib import Path

test/unit/config/test__base.py

@@ -15,6 +15,7 @@
 
 
 def load_file_side_effect(path, value=None, *args, **kwargs):
+    # pylint: disable=W0613
     if args[0] == path:
         if value:
             return value
@@ -36,6 +37,7 @@ def test_base_config_init_defaults(tmp_path):
 
 
 def test_base_config_with_artifact_dir(tmp_path, patch_private_data_dir):
+    # pylint: disable=W0613
     rc = BaseConfig(artifact_dir=tmp_path.joinpath('this-is-some-dir').as_posix())
     assert rc.artifact_dir == tmp_path.joinpath('this-is-some-dir').joinpath(rc.ident).as_posix()
 

test/unit/config/test_ansible_cfg.py

@@ -10,6 +10,7 @@
 
 
 def test_ansible_cfg_init_defaults(tmp_path, patch_private_data_dir):
+    # pylint: disable=W0613
     rc = AnsibleCfgConfig()
 
     # Check that the private data dir is placed in our default location with our default prefix

test/unit/config/test_command.py

@@ -9,6 +9,7 @@
 
 
 def test_ansible_config_defaults(tmp_path, patch_private_data_dir):
+    # pylint: disable=W0613
     rc = CommandConfig()
 
     # Check that the private data dir is placed in our default location with our default prefix

test/unit/config/test_container_volmount_generation.py

@@ -1,7 +1,7 @@
 """ Ensure the generation of container volume mounts is handled
 predictably and consistently """
 
-# pylint: disable=W0212
+# pylint: disable=W0212,W0621
 
 import os
 from typing import NamedTuple
@@ -60,13 +60,13 @@ def resolve_path(path):
     return os.path.abspath(os.path.expanduser(os.path.expandvars(path)))
 
 
-def generate_volmount_args(src_str, dst_str, labels):
+def generate_volmount_args(src_str, dst_str, vol_labels):
     """Generate a podman style volmount string"""
     vol_mount_str = f"{src_str}:{dst_str}"
-    if labels:
-        if not labels.startswith(":"):
+    if vol_labels:
+        if not vol_labels.startswith(":"):
             vol_mount_str += ":"
-        vol_mount_str += labels
+        vol_mount_str += vol_labels
     return ["-v", vol_mount_str]
 
 
@@ -95,12 +95,12 @@ def test_check_not_safe_to_mount_file(not_safe, mocker):
 
 @pytest.mark.parametrize("path", dir_variations, ids=id_for_src)
 def test_duplicate_detection_dst(path, mocker):
+    """Ensure no duplicate volume mount entries are created"""
     mocker.patch("os.path.exists", return_value=True)
     mocker.patch("os.path.isdir", return_value=True)
-    """Ensure no duplicate volume mount entries are created"""
     base_config = BaseConfig()
 
-    def generate(args_list):
+    def generate():
         for entry in dir_variations:
             for label in labels:
                 base_config._update_volume_mount_paths(
@@ -111,9 +111,9 @@ def generate(args_list):
                 )
 
     first_pass = []
-    generate(first_pass)
+    generate()
     second_pass = first_pass[:]
-    generate(second_pass)
+    generate()
     assert first_pass == second_pass
 
 
@@ -125,7 +125,7 @@ def test_no_dst_all_dirs(path, labels, mocker):
     """Ensure dst == src when not provided"""
     src_str = os.path.join(resolve_path(path.path), "")
     dst_str = src_str
-    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, labels=labels)
+    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, vol_labels=labels)
 
     result = []
     BaseConfig()._update_volume_mount_paths(
@@ -150,7 +150,7 @@ def test_src_dst_all_dirs(src, dst, labels, mocker):
     """Ensure src and dest end with trailing slash"""
     src_str = os.path.join(resolve_path(src.path), "")
     dst_str = os.path.join(resolve_path(dst.path), "")
-    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, labels=labels)
+    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, vol_labels=labels)
 
     result = []
     BaseConfig()._update_volume_mount_paths(
@@ -172,7 +172,7 @@ def test_src_dst_all_files(path, labels, mocker):
     """Ensure file paths are transformed correctly into dir paths"""
     src_str = os.path.join(resolve_path(path.path), "")
     dst_str = src_str
-    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, labels=labels)
+    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, vol_labels=labels)
 
     result = []
     src_file = os.path.join(path.path, "", "file.txt")
@@ -207,7 +207,7 @@ def test_src_dst_all_relative_dirs(src, dst, labels, relative, mocker):
     workdir = "/workdir"
     src_str = os.path.join(resolve_path(relative_src), "")
     dst_str = os.path.join(resolve_path(os.path.join(workdir, relative_dst)), "")
-    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, labels=labels)
+    expected = generate_volmount_args(src_str=src_str, dst_str=dst_str, vol_labels=labels)
 
     result = []
     BaseConfig(container_workdir=workdir)._update_volume_mount_paths(

test/unit/config/test_doc.py

@@ -10,6 +10,7 @@
 
 
 def test_ansible_doc_defaults(tmp_path, patch_private_data_dir):
+    # pylint: disable=W0613
     rc = DocConfig()
 
     # Check that the private data dir is placed in our default location with our default prefix

test/unit/config/test_inventory.py

@@ -10,6 +10,7 @@
 
 
 def test_ansible_inventory_init_defaults(tmp_path, patch_private_data_dir):
+    # pylint: disable=W0613
     rc = InventoryConfig()
 
     # Check that the private data dir is placed in our default location with our default prefix

test/unit/config/test_runner.py

@@ -16,6 +16,7 @@
 
 
 def load_file_side_effect(path, value=None, *args, **kwargs):
+    # pylint: disable=W0613
     if args[0] == path:
         if value:
             return value
@@ -373,8 +374,8 @@ def test_generate_ansible_command_with_dict_extravars(mocker):
 
 
 @pytest.mark.parametrize('cmdline,tokens', [
-    (u'--tags foo --skip-tags', ['--tags', 'foo', '--skip-tags']),
-    (u'--limit "䉪ቒ칸ⱷ?噂폄蔆㪗輥"', ['--limit', '䉪ቒ칸ⱷ?噂폄蔆㪗輥']),
+    ('--tags foo --skip-tags', ['--tags', 'foo', '--skip-tags']),
+    ('--limit "䉪ቒ칸ⱷ?噂폄蔆㪗輥"', ['--limit', '䉪ቒ칸ⱷ?噂폄蔆㪗輥']),
 ])
 def test_generate_ansible_command_with_cmdline_args(cmdline, tokens, mocker):
     mocker.patch('os.makedirs', return_value=True)
@@ -400,7 +401,7 @@ def test_prepare_command_defaults(mocker):
     cmd_side_effect = partial(load_file_side_effect, 'args')
 
     def generate_side_effect():
-        return StringIO(u'test "string with spaces"')
+        return StringIO('test "string with spaces"')
 
     mocker.patch.object(rc.loader, 'load_file', side_effect=cmd_side_effect)
     mocker.patch.object(rc, 'generate_ansible_command', side_effect=generate_side_effect)
@@ -552,6 +553,7 @@ def test_bwrap_process_isolation_defaults(mocker):
 
 
 def test_bwrap_process_isolation_and_directory_isolation(mocker, patch_private_data_dir, tmp_path):
+    # pylint: disable=W0613
 
     def mock_exists(path):
         if path == "/project":
@@ -565,7 +567,7 @@ def __init__(self, base_path):
         def load_file(self, path, objtype=None, encoding='utf-8'):
             raise ConfigurationError
 
-        def isfile(self, path):
+        def isfile(self, _):
             return False
 
     mocker.patch('ansible_runner.config.runner.os.makedirs', return_value=True)