Issue984/switch to pylint

.github/workflows/pylama.yml → .github/workflows/pylint.yml

@@ -1,4 +1,4 @@
-name: PyLama Lint
+name: Pylint Lint
 
 on:
   # For manual exec
@@ -11,13 +11,13 @@ on:
     branches: [main, develop]
 
 jobs:
-  pylama:
+  pylint:
     if: ${{ !(contains(github.event.pull_request.labels.*.name, 'WIP (no-ci)')) }}
-    name: PyLama Lint
+    name: Pylint Lint
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - name: Lint
         run: |
-          pip install pylama==8.4.1 pyflakes==3.0.1 pylint==2.15.9 pydocstyle==6.1.1 2>&1 >/dev/null
-          pylama beeflow/
+          pip install pylint==3.2.7 2>&1 >/dev/null
+          pylint --rcfile=setup.cfg beeflow/

.pre-commit-config.yaml

@@ -0,0 +1,14 @@
+repos:
+- repo: local
+  hooks:
+    - id: pylint
+      name: pylint
+      entry: pylint
+      language: system
+      types: [python]
+      args:
+        [
+          "-rn", # Only display messages
+          "-sn", # Don't display the score
+          "--rcfile=setup.cfg", # Link to the config file
+        ]

beeflow/client/bee_client.py

@@ -4,6 +4,11 @@
 This script provides a client interface to the user to manage workflows.
 Capablities include submitting, starting, listing, pausing and cancelling workflows.
 """
+
+# Disable W0511: This allows us to have TODOs in the code
+# Disable R1732: Significant code restructuring required to fix
+# pylint:disable=W0511,R1732
+
 import os
 import sys
 import logging
@@ -34,7 +39,7 @@
 from beeflow.common.db import bdb
 
 # Length of a shortened workflow ID
-short_id_len = 6 #noqa: Not a constant
+short_id_len = 6 # pylint: disable=C0103 # not a constant
 
 # Maximum length of a workflow ID
 MAX_ID_LEN = 32
@@ -167,7 +172,7 @@ def error_exit(msg, include_caller=True):
         raise ClientError(msg) from None
 
 
-def error_handler(resp):  # noqa (this is an error handler, it doesn't need to return an expression)
+def error_handler(resp):  # pylint: disable=R1710 # error handler doesn't need to return an expression
     """Handle a 500 error in a response."""
     if resp.status_code != 500:
         return resp
@@ -231,7 +236,7 @@ def get_wf_list():
 
 def check_short_id_collision():
     """Check short workflow IDs for colliions; increase short ID length if detected."""
-    global short_id_len  #noqa: Not a constant
+    global short_id_len
     workflow_list = get_wf_list()
     if workflow_list:
         while short_id_len < MAX_ID_LEN:
@@ -470,13 +475,13 @@ def package(wf_path: pathlib.Path = typer.Argument(...,
     # Just use tar with subprocess. Python's tar library is not performant.
     return_code = subprocess.run(['tar', '-C', parent_dir, '-czf', tarball, wf_dir],
                                  check=True).returncode
-    package_path = package_dest.resolve()/tarball  # noqa: Not an arithmetic operation
+    package_path = package_dest.resolve()/tarball
 
     # Get the curent working directory
     cwd = pathlib.Path().absolute()
     if package_dest != cwd:
         # Move the tarball if the directory it's wanted in is not in the current working directory
-        tarball_path = cwd/tarball  # noqa: Not an arithmetic operation
+        tarball_path = cwd/tarball
         shutil.move(tarball_path, package_path)
 
     if return_code != 0:
@@ -700,7 +705,7 @@ def reexecute(wf_name: str = typer.Argument(..., help='The workflow name'),
     except requests.exceptions.ConnectionError:
         error_exit('Could not reach WF Manager.')
 
-    if resp.status_code != requests.codes.created: #noqa: member does exist
+    if resp.status_code != requests.codes.created: # pylint: disable=E1101
         error_exit(f"Reexecute for {wf_name} failed. Please check the WF Manager.")
 
     wf_id = resp.json()['wf_id']
@@ -765,7 +770,3 @@ def main():
 
 if __name__ == "__main__":
     app()
-
-# Ignore W0511: This allows us to have TODOs in the code
-# Ignore R1732: Significant code restructuring required to fix
-# pylama:ignore=W0511,R1732

beeflow/client/core.py

@@ -111,7 +111,7 @@ def poll(self):
         """Poll each process to check for errors, restart failed processes."""
         # Max number of times a component can be restarted
         max_restarts = bc.get('DEFAULT', 'max_restarts')
-        for name in self.procs:  # noqa no need to iterate with items() since self.procs may be set
+        for name in self.procs:  # pylint: disable=C0206 # no need to iterate with items() since self.procs may be set
             component = self.components[name]
             if component['failed']:
                 continue
@@ -255,7 +255,7 @@ def start_slurm_restd():
             # slurm_args = f'-s openapi/{openapi_version},openapi/db{openapi_version}'
             slurm_socket = paths.slurm_socket()
             subprocess.run(['rm', '-f', slurm_socket], check=True)
-            fp = open(slurmrestd_log, 'w', encoding='utf-8') # noqa
+            fp = open(slurmrestd_log, 'w', encoding='utf-8') # pylint: disable=R1732
             cmd = ['slurmrestd']
             cmd.extend(slurm_args.split())
             cmd.append(f'unix:{slurm_socket}')
@@ -277,7 +277,7 @@ def load_check_charliecloud():
     if not shutil.which('ch-run'):
         lmod = os.environ.get('MODULESHOME')
         sys.path.insert(0, lmod + '/init')
-        from env_modules_python import module #noqa No need to import at top
+        from env_modules_python import module # pylint: disable=C0415 # No need to import at top
         module("load", "charliecloud")
         # Try loading the Charliecloud module then test again
         if not shutil.which('ch-run'):
@@ -328,7 +328,7 @@ def check_dependencies(backend=False):
     # Check for the flux API
     if bc.get('DEFAULT', 'workload_scheduler') == 'Flux':
         try:
-            import flux  # noqa needed to check whether flux api is actually installed
+            import flux  # pylint: disable=W0611,C0415 # don't need to check whether flux api is actually installed
         except ModuleNotFoundError:
             warn('Failed to import flux Python API. Please make sure you can '
                  'use flux in your environment.')
@@ -382,7 +382,7 @@ def handle_client(self, server):
 
 def daemonize(mgr, base_components):
     """Start beeflow as a daemon, monitoring all processes."""
-    def handle_terminate(signum, stack): # noqa
+    def handle_terminate(signum, stack): # pylint: disable=W0613
         """Handle a terminate signal."""
         # Kill all subprocesses
         mgr.kill()

beeflow/cloud_launcher.py

@@ -1,4 +1,9 @@
 """BEE Cloud Installer Script."""
+
+# Disable R1732: Significant code restructuring required to fix
+# Disable W0511: This allows us to have TODOs in the code
+# pylint:disable=W0511,R1732
+
 import argparse
 import subprocess
 import sys
@@ -205,6 +210,3 @@ def main():
 
 if __name__ == '__main__':
     main()
-# Ignore R1732: Significant code restructuring required to fix
-# Ignore W0511: This allows us to have TODOs in the code
-# pylama:ignore=W0511,R1732

beeflow/common/api.py

@@ -7,6 +7,6 @@
 class BeeApi(Api):
     """Wrapper around Flask-Restful's API to catch exceptions."""
 
-    def handle_error(self, e):  # noqa (conflict on naming in base class vs. following convention)
+    def handle_error(self, e):  # pylint: disable=W0613 # conflict on naming in base class vs. following convention
         """Handle an error or exception."""
         return make_response(jsonify(error=traceback.format_exc()), 500)

beeflow/common/build/container_drivers.py

@@ -3,6 +3,10 @@
 All container-based build systems belong here.
 """
 
+# Disable W0231: linter doesn't know about abstract classes,
+# it's ok to now call the parent __init__
+# pylint:disable=W0231
+
 import os
 import shutil
 import subprocess
@@ -65,14 +69,14 @@ def __init__(self, task):
         try:
             requirement_docker_requirements = self.task.requirements['DockerRequirement'].keys()
             docker_requirements = docker_requirements.union(requirement_docker_requirements)
-            req_string = (f'{set(requirement_docker_requirements)}')
+            req_string = f'{set(requirement_docker_requirements)}'
             log.info(f'task {self.task.id} requirement DockerRequirements: {req_string}')
         except (TypeError, KeyError):
             log.info(f'task {self.task.name} {self.task.id} no DockerRequirements in requirement')
         try:
             hint_docker_requirements = self.task.hints['DockerRequirement'].keys()
             docker_requirements = docker_requirements.union(hint_docker_requirements)
-            hint_str = (f'{set(hint_docker_requirements)}')
+            hint_str = f'{set(hint_docker_requirements)}'
             log.info(f'task {self.task.name} {self.task.id} hint DockerRequirements: {hint_str}')
         except (TypeError, KeyError):
             log.info(f'task {self.task.name} {self.task.id} hints has no DockerRequirements')
@@ -252,7 +256,7 @@ def process_docker_import(self, param_import=None):
 
         # Pull the image.
         file_name = crt_driver.get_ccname(import_input_path)
-        cmd = (f'ch-convert {import_input_path} {self.deployed_image_root}/{file_name}')
+        cmd = f'ch-convert {import_input_path} {self.deployed_image_root}/{file_name}'
         log.info(f'Docker import: Assuming container name is {import_input_path}. Correct?')
         return subprocess.run(cmd, check=True, shell=True)
 
@@ -423,5 +427,3 @@ def process_docker_output_directory(self, param_output_directory=None):
         param_output_directory may be used to override DockerRequirement
         specs.
         """
-# Ignore W0231: linter doesn't know about abstract classes, it's ok to now call the parent __init__
-# pylama:ignore=W0231

beeflow/common/build_interfaces.py

@@ -6,6 +6,9 @@
 components of the gdb_interface as required.
 """
 
+# Disable W0703: Catching generic exception isn't a problem if we just want a descriptive report
+# pylint:disable=W0703
+
 # from beeflow.common.gdb.gdb_interface import GraphDatabaseInterface
 # from beeflow.common.build.container_drivers import CharliecloudBuildDriver,
 #                                                    SingularityBuildDriver
@@ -82,7 +85,3 @@ def build_main(task):
         log.info(f'{err}')
 
     build_main(local_task)
-
-# Ignore W0703: Catching generic exception isn't a problem if we just want a descriptive report
-# Ignore C901: "'build_main' is too complex" - this function is just around 40 lines
-# pylama:ignore=W0703,C901

beeflow/common/cli.py

@@ -5,6 +5,6 @@
 class NaturalOrderGroup(click.Group):
     """Natural ordering class for using with CLI code."""
 
-    def list_commands(self, ctx):  # noqa
+    def list_commands(self, ctx):
         """List the commands in order."""
         return self.commands.keys()

beeflow/common/cloud/__init__.py

@@ -1,4 +1,8 @@
 """Cloud init module."""
+
+# Disable W0611: These are meant to be used by external code
+# pylint:disable=W0611
+
 from beeflow.common.cloud import chameleoncloud
 from beeflow.common.cloud import openstack
 from beeflow.common.cloud import provider
@@ -21,5 +25,3 @@ def get_provider(name, **kwargs):
         return providers[name](**kwargs)
 
     raise RuntimeError(f'Invalid provider "{name}"')
-# Ignore W0611: These are meant to be used by external code
-# pylama:ignore=W0611

beeflow/common/cloud/chameleoncloud.py

@@ -19,7 +19,7 @@ def create_from_template(self, template_file):
             'Use the Horizon interface instead'
         )
 
-    def get_ext_ip_addr(self, node_name):  # noqa
+    def get_ext_ip_addr(self, node_name):  # pylint: disable=W0613
         """Get the external IP address of the node, if it has one."""
         if self._stack_name is not None:
             stack = self._api.get_stack(self._stack_name)

beeflow/common/cloud/google.py

@@ -19,7 +19,7 @@ def __init__(self, project, zone, **kwargs):
 
     def get_ext_ip_addr(self, node_name):
         """Get the external IP of this node (or None if no IP)."""
-        res = self._api.instances().get(instance=node_name,  # noqa (can't find instances member)
+        res = self._api.instances().get(instance=node_name,
                                         project=self.project,
                                         zone=self.zone).execute()
         try:
@@ -34,7 +34,7 @@ def setup_cloud(self, config):
         # This just creates instances one-by-one. There may be a better API call
         # to just create everything at once.
         for instance in config['instances']:
-            call = self._api.instances().insert(project=self.project,  # noqa (can't find instances member)
+            call = self._api.instances().insert(project=self.project,
                                                 zone=self.zone, body=instance)
             res = call.execute()
             print(res)

beeflow/common/config_driver.py

@@ -126,7 +126,7 @@ def get(cls, sec_name, opt_name):
         if cls.CONFIG is None:
             cls.init()
         try:
-            return cls.CONFIG[sec_name][opt_name] # noqa (this object is subscritable)
+            return cls.CONFIG[sec_name][opt_name] # pylint: disable=E1136 # object is subscritable
         except KeyError:
             raise RuntimeError(
                 f'Option {sec_name}::{opt_name} was not found. Please contact '
@@ -359,7 +359,7 @@ def validate_chrun_opts(opts):
                  info='scheduling algorithm to use', prompt=False)
 VALIDATOR.option('scheduler', 'default_algorithm', default='fcfs',
                  choices=SCHEDULER_ALGORITHMS, prompt=False,
-                 info=('default algorithm to use'))
+                 info='default algorithm to use')
 
 
 def print_wrap(text, next_line_indent=''):
@@ -605,7 +605,3 @@ def show(path: str = typer.Argument(default=USERCONFIG_FILE,
     print(f'# {path}')
     with open(path, encoding='utf-8') as fp:
         print(fp.read(), end='')
-# Ignore C901: "'ConfigGenerator.choose_values' is too complex" - I disagree, if
-#              it's just based on LOC, then there are a number `print()` functions
-#              that are increasing the line count
-# pylama:ignore=C901

beeflow/common/config_utils.py

@@ -8,7 +8,7 @@ def filter_and_validate(config, validator):
     """Filter and validate the configuration file."""
     default_keys = list(config['DEFAULT'])
     config = {sec_name: {key: config[sec_name][key] for key in config[sec_name]
-                         if sec_name == 'DEFAULT' or key not in default_keys} # noqa
+                         if sec_name == 'DEFAULT' or key not in default_keys}
               for sec_name in config}
     # Validate the config
     return validator.validate(config)

beeflow/common/connection.py

@@ -27,7 +27,7 @@ def _full_url(self, path):
 
     def handle_error(self, resp):
         """Handle an error, if there is one."""
-        if resp.status_code != requests.codes.okay:  # noqa (pylama can't find the okay member)
+        if resp.status_code != requests.codes.okay:  # pylint: disable=E1101 # pylint can't find the okay member
             return self._error_handler(resp)
         return resp
 

beeflow/common/crt/charliecloud_driver.py

@@ -42,7 +42,7 @@ def get_ccname(image_path):
         name = '.'.join(name)
         return name
 
-    def run_text(self, task):  # noqa
+    def run_text(self, task):  # pylint: disable=R0915
         """Create text for Charliecloud batch script."""
         os.makedirs(self.container_archive, exist_ok=True)
         log.info(f'Build container archive directory is: {self.container_archive}')
@@ -148,5 +148,5 @@ def run_text(self, task):  # noqa
     def build_text(self, userconfig, task):
         """Build text for Charliecloud batch script."""
         task_args = task2arg(task)
-        text = (f'beeflow --build {userconfig} {task_args}\n')
+        text = f'beeflow --build {userconfig} {task_args}\n'
         return text

beeflow/common/crt_interface.py

@@ -5,6 +5,9 @@
 Default: 'CharliecloudDriver' class.
 """
 
+# Disable module imported but unused error. No way to know which crt will be needed
+# pylint:disable=W0611
+
 from beeflow.common.config_driver import BeeConfig as bc
 from beeflow.common.crt.charliecloud_driver import CharliecloudDriver
 from beeflow.common.crt.singularity_driver import SingularityDriver
@@ -40,5 +43,3 @@ def build_text(self, userconfig, task):
         :rtype: string
         """
         return self._crt_driver.build_text(userconfig, task)
-# Ignore module imported but unused error. No way to know which crt will be needed
-# pylama:ignore=W0611

beeflow/common/db/client_db.py

@@ -10,7 +10,7 @@ class ClientInfo:
 
     def __init__(self, db_file):
         """Initialize info and db file."""
-        self.Info = namedtuple("Info", "id hostname") # noqa Snake Case
+        self.Info = namedtuple("Info", "id hostname") # pylint: disable=C0103
         self.db_file = db_file
 
     def set_hostname(self, new_hostname):