Skip to content

Commit

Permalink
Pylint and flake8
Browse files Browse the repository at this point in the history
  • Loading branch information
HebaruSan committed Jan 29, 2021
1 parent 7fdf5c1 commit 56d9de2
Show file tree
Hide file tree
Showing 30 changed files with 143 additions and 114 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ jobs:
run: pip install .[test]
- name: Run pytest
working-directory: netkan
run: pytest --mypy -v
run: pytest -v
8 changes: 8 additions & 0 deletions netkan/.flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[flake8]
ignore =
# Whitespace before colons
E203,
# Whitespace after colons
E241,
# Line too long
E501,
13 changes: 13 additions & 0 deletions netkan/.pylintrc
Original file line number Diff line number Diff line change
@@ -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,
2 changes: 1 addition & 1 deletion netkan/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions netkan/netkan/auto_freezer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -62,17 +61,19 @@ 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:
@staticmethod
def _timestamp_before(dttm: Optional[datetime], update_cutoff: datetime) -> bool:
return dttm < update_cutoff if dttm else False

def _add_freezee(self, ident: str) -> None:
Expand Down Expand Up @@ -101,6 +102,5 @@ def _submit_pr(self, branch_name: str, days: int, idle_mods: List[Tuple[str, dat
title='Freeze idle mods',
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)),
'\n\n' + self._mod_table(idle_mods)),
)
2 changes: 1 addition & 1 deletion netkan/netkan/cli/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions netkan/netkan/cli/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions netkan/netkan/cli/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 9 additions & 1 deletion netkan/netkan/common.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
}
6 changes: 4 additions & 2 deletions netkan/netkan/download_counter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions netkan/netkan/github_pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ 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']
)
5 changes: 2 additions & 3 deletions netkan/netkan/indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 = [
Expand Down
72 changes: 34 additions & 38 deletions netkan/netkan/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from .csharp_compat import csharp_uri_tostring


class Netkan:

KREF_PATTERN = re.compile('^#/ckan/([^/]+)/(.+)$')
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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'
Expand All @@ -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

Expand Down Expand Up @@ -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]

0 comments on commit 56d9de2

Please sign in to comment.