Pylint and flake8

.github/workflows/test.yml

@@ -18,4 +18,4 @@ jobs:
         run: pip install .[test]
       - name: Run pytest
         working-directory: netkan
-        run: pytest --mypy -v
+        run: pytest -v

.vscode/settings.json

@@ -1,7 +1,7 @@
 {
   "python.pythonPath": "/usr/bin/python3",
   "python.formatting.provider": "autopep8",
-  "editor.formatOnSave": false,
+  "editor.formatOnSave": true,
   "python.linting.pylintEnabled": true,
   "python.linting.enabled": true,
   "python.linting.pylintArgs": [
@@ -13,7 +13,6 @@
   "files.exclude": {
     "**/__pycache__": true
   },
-  "python.testing.pytestArgs": ["--verbose", "--exitfirst", "netkan/tests"],
   "python.testing.pytestEnabled": true,
   "python.testing.nosetestsEnabled": false,
   "python.testing.unittestEnabled": false,

netkan/.flake8

@@ -0,0 +1,8 @@
+[flake8]
+ignore =
+    # Whitespace before colons
+    E203,
+    # Whitespace after colons
+    E241,
+    # Line too long
+    E501,

netkan/.pylintrc

@@ -0,0 +1,13 @@
+[MESSAGES CONTROL]
+disable =
+    line-too-long,
+    duplicate-code,
+    missing-module-docstring,
+    missing-class-docstring,
+    missing-function-docstring,
+    too-few-public-methods,
+    too-many-instance-attributes,
+    too-many-statements,
+    too-many-return-statements,
+    too-many-branches,
+    too-many-arguments,

netkan/Dockerfile

@@ -27,7 +27,7 @@ RUN chown -R netkan:netkan /netkan
 USER netkan
 WORKDIR /netkan
 RUN pip install --user .[test]
-RUN /home/netkan/.local/bin/pytest --mypy -v
+RUN /home/netkan/.local/bin/pytest -v
 
 FROM production as dev
 USER root

netkan/netkan/auto_freezer.py

@@ -49,8 +49,7 @@ def _checkout_branch(self, name: str) -> None:
         except git.GitCommandError:
             logging.info('Unable to fetch %s', name)
 
-        (getattr(self.nk_repo.git_repo.heads, name, None)
-         or self.nk_repo.git_repo.create_head(name)).checkout()
+        (getattr(self.nk_repo.git_repo.heads, name, None) or self.nk_repo.git_repo.create_head(name)).checkout()
 
     def _ids(self) -> Iterable[str]:
         return (nk.identifier for nk in self.nk_repo.netkans())
@@ -62,19 +61,17 @@ def _find_idle_mods(self, days_limit: int, days_till_ignore: int) -> List[Tuple[
         idle_mods = []
         for ident in self._ids():
             dttm = self._last_timestamp(ident)
-            if dttm and dttm < update_cutoff and dttm > too_old_cutoff:
+            if dttm and too_old_cutoff < dttm < update_cutoff:
                 idle_mods.append((ident, dttm))
         return idle_mods
 
-    def _last_timestamp(self, ident: str) -> Optional[datetime]:
+    @staticmethod
+    def _last_timestamp(ident: str) -> Optional[datetime]:
         status = ModStatus.get(ident)
         return getattr(status, 'release_date',
                        getattr(status, 'last_indexed',
                                None))
 
-    def _timestamp_before(self, dttm: Optional[datetime], update_cutoff: datetime) -> bool:
-        return dttm < update_cutoff if dttm else False
-
     def _add_freezee(self, ident: str) -> None:
         self.nk_repo.git_repo.index.move([
             self.nk_repo.nk_path(ident).as_posix(),
@@ -102,5 +99,5 @@ def _submit_pr(self, branch_name: str, days: int, idle_mods: List[Tuple[str, dat
                 body=(f'The attached mods have not updated in {days} or more days.'
                       ' Freeze them to save the bot some CPU cycles.'
                       '\n\n'
-                      + self._mod_table(idle_mods)),
+                      f'{self._mod_table(idle_mods)}'),
             )

netkan/netkan/cli/common.py

@@ -53,7 +53,7 @@ def ctx_callback(ctx: click.Context, param: click.Parameter,
 ]
 
 
-class SharedArgs(object):
+class SharedArgs:
 
     def __init__(self) -> None:
         self._environment_data = None

netkan/netkan/cli/services.py

@@ -58,8 +58,8 @@ def indexer(common: SharedArgs) -> None:
 def scheduler(common: SharedArgs, max_queued: int, group: str, min_cpu: int, min_io: int) -> None:
     sched = NetkanScheduler(
         common.netkan_repo, common.ckanmeta_repo, common.queue, common.token,
-        nonhooks_group=(group == 'all' or group == 'nonhooks'),
-        webhooks_group=(group == 'all' or group == 'webhooks'),
+        nonhooks_group=(group in ('all', 'nonhooks')),
+        webhooks_group=(group in ('all', 'webhooks')),
     )
     if sched.can_schedule(max_queued, common.dev, min_cpu, min_io):
         sched.schedule_all_netkans()

netkan/netkan/cli/utilities.py

@@ -115,14 +115,14 @@ def redeploy_service(cluster: str, service_name: str) -> None:
                                     cluster=cluster)['serviceArns']
     try:
         service = list(filter(lambda i: service_name in i, services))[0]
-    except IndexError:
+    except IndexError as exc:
         available = '\n    - '.join(
             [f.split('/')[1].split('-')[1] for f in services]
         )
         raise click.UsageError(
             "Service '{}' not found. Available services:\n    - {}".format(
                 service_name, available)
-        )
+        ) from exc
     client.update_service(
         cluster=cluster,
         service=service,

netkan/netkan/common.py

@@ -1,4 +1,5 @@
-from typing import List, Iterable, Dict
+from typing import List, Iterable, Dict, Any
+import boto3  # pylint: disable=unused-import
 
 import github
 from git import Repo
@@ -31,3 +32,10 @@ def pull_all(repos: Iterable[Repo]) -> None:
 
 def github_limit_remaining(token: str) -> int:
     return github.Github(token).get_rate_limit().core.remaining
+
+
+def deletion_msg(msg: 'boto3.resources.factory.sqs.Message') -> Dict[str, Any]:
+    return {
+        'Id':            msg.message_id,
+        'ReceiptHandle': msg.receipt_handle,
+    }

netkan/netkan/download_counter.py

@@ -151,7 +151,8 @@ def get_module_query(self, nk_dl: NetkanDownloads) -> Optional[str]:
                     user=user, repo=repo)
         return None
 
-    def graphql_safe_identifier(self, nk_dl: NetkanDownloads) -> str:
+    @staticmethod
+    def graphql_safe_identifier(nk_dl: NetkanDownloads) -> str:
         """
         Identifiers can start with numbers and include hyphens.
         GraphQL doesn't like that, so we put an 'x' on the front
@@ -160,7 +161,8 @@ def graphql_safe_identifier(self, nk_dl: NetkanDownloads) -> str:
         """
         return f'x{nk_dl.identifier.replace("-", "_")}'
 
-    def from_graphql_safe_identifier(self, fake_ident: str) -> str:
+    @staticmethod
+    def from_graphql_safe_identifier(fake_ident: str) -> str:
         """
         Inverse of the above. Strip off the first character and
         replace underscores with dashes, to get back to the original

netkan/netkan/github_pr.py

@@ -38,12 +38,8 @@ def create_pull_request(self, title: str, branch: str, body: str) -> None:
             except KeyError:
                 pass
             logging.error('PR for %s failed: %s - %s',
-                branch,
-                message,
-                error
-            )
+                          branch, message, error)
             return
         pr_json = response.json()
         logging.info('PR for %s opened at %s',
-            branch, pr_json['html_url']
-        )
+                     branch, pr_json['html_url'])

netkan/netkan/indexer.py

@@ -36,7 +36,7 @@ def __init__(self, msg: 'boto3.resources.factory.sqs.Message',
             attr_type = '{}Value'.format(item[1]['DataType'])
             content = item[1][attr_type]
             if content.lower() in ['true', 'false']:
-                content = True if content.lower() == 'true' else False
+                content = (content.lower() == 'true')
             if item[0] == 'FileName':
                 content = PurePath(content).name
             setattr(self, item[0], content)
@@ -123,8 +123,7 @@ def _process_ckan(self) -> None:
             if not self.Success and getattr(status, 'last_error', None) != self.ErrorMessage:
                 logging.error('New inflation error for %s: %s',
                               self.ModIdentifier, self.ErrorMessage)
-            elif (getattr(status, 'last_warnings', None) != self.WarningMessages and
-                  self.WarningMessages is not None):
+            elif (getattr(status, 'last_warnings', None) != self.WarningMessages and self.WarningMessages is not None):
                 logging.error('New inflation warnings for %s: %s',
                               self.ModIdentifier, self.WarningMessages)
             actions = [

netkan/netkan/metadata.py

@@ -10,6 +10,7 @@
 
 from .csharp_compat import csharp_uri_tostring
 
+
 class Netkan:
 
     KREF_PATTERN = re.compile('^#/ckan/([^/]+)/(.+)$')
@@ -67,7 +68,8 @@ def hook_only(self) -> bool:
             return False
         return self.on_spacedock
 
-    def string_attrib(self, val: str) -> Dict[str, str]:
+    @staticmethod
+    def string_attrib(val: str) -> Dict[str, str]:
         return {
             'DataType': 'String',
             'StringValue': val,
@@ -123,29 +125,29 @@ def __eq__(self, other: object) -> bool:
         # (except __eq__) are deduced from it by @total_ordering.
         def __gt__(self, other: 'Ckan.Version') -> bool:
 
-            def _string_compare(v1: str, v2: str) -> Tuple[int, str, str]:
+            def _string_compare(ver1: str, ver2: str) -> Tuple[int, str, str]:
                 _result: int
                 _first_remainder = ''
                 _second_remainder = ''
 
                 # Our starting assumptions are, that both versions are completely strings,
                 # with no remainder. We'll then check if they're not.
-                str1 = v1
-                str2 = v2
+                str1 = ver1
+                str2 = ver2
 
                 # Start by walking along our version string until we find a number,
                 # thereby finding the starting string in both cases. If we fall off
                 # the end, then our assumptions made above hold.
-                for i in range(0, len(v1)):
-                    if v1[i].isdigit():
-                        _first_remainder = v1[i:]
-                        str1 = v1[:i]
+                for i, piece in enumerate(ver1):
+                    if piece.isdigit():
+                        _first_remainder = ver1[i:]
+                        str1 = ver1[:i]
                         break
 
-                for i in range(0, len(v2)):
-                    if v2[i].isdigit():
-                        _second_remainder = v2[i:]
-                        str2 = v2[:i]
+                for i, piece in enumerate(ver2):
+                    if piece.isdigit():
+                        _second_remainder = ver2[i:]
+                        str2 = ver2[:i]
                         break
 
                 # Then compare the two strings, and return our comparison state.
@@ -164,42 +166,42 @@ def _string_compare(v1: str, v2: str) -> Tuple[int, str, str]:
                             _result = 0
                     else:
                         # Do an artificial __cmp__
-                        _result = ((str1 > str2)-(str1 < str2))
+                        _result = ((str1 > str2) - (str1 < str2))
                 else:
-                    _result = ((str1 > str2)-(str1 < str2))
+                    _result = ((str1 > str2) - (str1 < str2))
 
                 return _result, _first_remainder, _second_remainder
 
-            def _number_compare(v1: str, v2: str) -> Tuple[int, str, str]:
+            def _number_compare(ver1: str, ver2: str) -> Tuple[int, str, str]:
                 _result: int
                 _first_remainder = ''
                 _second_remainder = ''
 
                 minimum_length1 = 0
-                for i in range(0, len(v1)):
-                    if not v1[i].isdigit():
-                        _first_remainder = v1[i:]
+                for i, piece in enumerate(ver1):
+                    if not piece.isdigit():
+                        _first_remainder = ver1[i:]
                         break
                     minimum_length1 += 1
 
                 minimum_length2 = 0
-                for i in range(0, len(v2)):
-                    if not v2[i].isdigit():
-                        _second_remainder = v2[i:]
+                for i, piece in enumerate(ver2):
+                    if not piece.isdigit():
+                        _second_remainder = ver2[i:]
                         break
                     minimum_length2 += 1
 
-                if v1[:minimum_length1].isnumeric():
-                    integer1 = int(v1[:minimum_length1])
+                if ver1[:minimum_length1].isnumeric():
+                    integer1 = int(ver1[:minimum_length1])
                 else:
                     integer1 = 0
 
-                if v2[:minimum_length2].isnumeric():
-                    integer2 = int(v2[:minimum_length2])
+                if ver2[:minimum_length2].isnumeric():
+                    integer2 = int(ver2[:minimum_length2])
                 else:
                     integer2 = 0
 
-                _result = ((integer1 > integer2)-(integer1 < integer2))
+                _result = ((integer1 > integer2) - (integer1 < integer2))
                 return _result, _first_remainder, _second_remainder
 
             # Here begins the main comparison logic
@@ -249,10 +251,10 @@ def __str__(self) -> str:
 
     CACHE_PATH = Path.home().joinpath('ckan_cache')
     MIME_TO_EXTENSION = {
-        'application/x-gzip':           'gz',
-        'application/x-tar':            'tar',
+        'application/x-gzip': 'gz',
+        'application/x-tar': 'tar',
         'application/x-compressed-tar': 'tar.gz',
-        'application/zip':              'zip',
+        'application/zip': 'zip',
     }
     ISODATETIME_PROPERTIES = [
         'release_date'
@@ -272,7 +274,7 @@ def _custom_parser(self, dct: Dict[str, Any]) -> Dict[str, Any]:
             if k in dct:
                 try:
                     dct[k] = dateutil.parser.isoparse(dct[k])
-                except: # pylint: disable=bare-except
+                except:  # pylint: disable=bare-except  # noqa: E722
                     pass
         return dct
 
@@ -340,14 +342,8 @@ def source_download(self, branch: str = 'master') -> Optional[str]:
 
     def authors(self) -> List[str]:
         auth = self.author
-        if isinstance(auth, list):
-            return auth
-        else:
-            return [auth]
+        return auth if isinstance(auth, list) else [auth]
 
     def licenses(self) -> List[str]:
         lic = self.license
-        if isinstance(lic, list):
-            return lic
-        else:
-            return [lic]
+        return lic if isinstance(lic, list) else [lic]