From f676f892b122f9092d21af03ad761dbdf5d1932e Mon Sep 17 00:00:00 2001 From: Ricardo Branco Date: Mon, 3 Jul 2023 11:19:36 +0200 Subject: [PATCH 1/2] Minor fixes --- Makefile | 9 ++++----- ocw/apps.py | 14 +++++++------- ocw/enums.py | 9 ++++----- ocw/lib/db.py | 3 --- ocw/lib/openstack.py | 2 +- ocw/models.py | 10 +++++----- ocw/tables.py | 25 ++++++++++++------------- ocw/views.py | 21 ++++++++++++--------- pylintrc | 1 - tests/kubernetes.py | 4 +++- tests/test_azure.py | 4 +++- tests/test_gce.py | 4 +++- webui/PCWConfig.py | 8 +++++--- webui/settings.py | 6 ++---- 14 files changed, 61 insertions(+), 59 deletions(-) diff --git a/Makefile b/Makefile index 78e044b3..91931ca5 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,7 @@ # Default container tag CONT_TAG=suse/qac/pcw +LINE_MAX=140 +FILES=ocw/lib/*.py ocw/management/commands/*.py ocw/*.py *.py .PHONY: all all: prepare flake8 test pylint @@ -10,15 +12,12 @@ prepare: .PHONY: pylint pylint: - pylint ocw/lib/*.py cleanup_k8s.py + pylint $(FILES) -LINE_MAX=140 .PHONY: flake8 flake8: - flake8 --max-line-length=$(LINE_MAX) webui - flake8 --max-line-length=$(LINE_MAX) ocw + flake8 --max-line-length=$(LINE_MAX) $(FILES) flake8 --max-line-length=$(LINE_MAX) manage.py - flake8 --max-line-length=$(LINE_MAX) cleanup_k8s.py .PHONY: test test: diff --git a/ocw/apps.py b/ocw/apps.py index 13328348..b826a032 100644 --- a/ocw/apps.py +++ b/ocw/apps.py @@ -1,17 +1,17 @@ -from django.apps import AppConfig import os import logging +from django.apps import AppConfig from apscheduler.schedulers.background import BackgroundScheduler from apscheduler.executors.pool import ThreadPoolExecutor from pytz import utc logger = logging.getLogger(__name__) -__scheduler = None +__SCHEDULER = None -def getScheduler(): - global __scheduler - if __scheduler is None: +def getScheduler(): # pylint: disable=invalid-name + global __SCHEDULER + if __SCHEDULER is None: logger.info("Create new BackgrounScheduler") executors = { 'default': ThreadPoolExecutor(1), @@ -20,8 +20,8 @@ def getScheduler(): 'coalesce': False, 'max_instances': 1 } - __scheduler = BackgroundScheduler(executors=executors, job_defaults=job_defaults, timezone=utc) - return __scheduler + __SCHEDULER = BackgroundScheduler(executors=executors, job_defaults=job_defaults, timezone=utc) + return __SCHEDULER class OcwConfig(AppConfig): diff --git a/ocw/enums.py b/ocw/enums.py index 4a3870ce..6523ba57 100644 --- a/ocw/enums.py +++ b/ocw/enums.py @@ -26,14 +26,13 @@ class ProviderChoice(ChoiceEnum): def from_str(provider): if provider.upper() == ProviderChoice.GCE: return ProviderChoice.GCE - elif provider.upper() == ProviderChoice.EC2: + if provider.upper() == ProviderChoice.EC2: return ProviderChoice.EC2 - elif provider.upper() == ProviderChoice.AZURE: + if provider.upper() == ProviderChoice.AZURE: return ProviderChoice.AZURE - elif provider.upper() == ProviderChoice.OSTACK: + if provider.upper() == ProviderChoice.OSTACK: return ProviderChoice.OSTACK - else: - raise ValueError(f"{provider} is not convertable to ProviderChoice") + raise ValueError(f"{provider} is not convertable to ProviderChoice") class StateChoice(ChoiceEnum): diff --git a/ocw/lib/db.py b/ocw/lib/db.py index 91ebffb3..9e987335 100644 --- a/ocw/lib/db.py +++ b/ocw/lib/db.py @@ -202,17 +202,14 @@ def auto_delete_instances() -> None: def is_updating(): - global RUNNING return RUNNING def last_update(): - global LAST_UPDATE return LAST_UPDATE if LAST_UPDATE is not None else '' def start_update(): - global RUNNING if not RUNNING: getScheduler().get_job('update_db').reschedule(trigger='date', run_date=datetime.now(timezone.utc)) diff --git a/ocw/lib/openstack.py b/ocw/lib/openstack.py index 33fcd9de..2915b484 100644 --- a/ocw/lib/openstack.py +++ b/ocw/lib/openstack.py @@ -24,7 +24,7 @@ def __new__(cls, vault_namespace: str): def client(self) -> None: if self.__client is None: self.__client = openstack.connect( - debug=DEBUG, + debug=bool(DEBUG), insecure=True, # Trust the certificate auth_url=self.get_data('auth_url'), project_name=self.get_data('project_name'), diff --git a/ocw/models.py b/ocw/models.py index 6ba044e4..a205750a 100644 --- a/ocw/models.py +++ b/ocw/models.py @@ -1,7 +1,7 @@ -from django.db import models +import json from datetime import datetime, timedelta, timezone +from django.db import models from webui.PCWConfig import PCWConfig -import json from .enums import ProviderChoice, StateChoice @@ -42,9 +42,9 @@ def all_time_fields(self): all_time_pattern = "(age={}, first_seen={}, last_seen={}, ttl={})" first_fmt = 'None' last_fmt = 'None' - if (self.first_seen): + if self.first_seen: first_fmt = self.first_seen.strftime('%Y-%m-%d %H:%M') - if (self.last_seen): + if self.last_seen: last_fmt = self.last_seen.strftime('%Y-%m-%d %H:%M') return all_time_pattern.format(self.age_formated(), first_fmt, last_fmt, self.ttl_formated()) @@ -59,7 +59,7 @@ def set_alive(self): def get_type(self): return self.cspinfo.type - class Meta: + class Meta: # pylint: disable=too-few-public-methods unique_together = (('provider', 'instance_id', 'vault_namespace'),) diff --git a/ocw/tables.py b/ocw/tables.py index bc5f8280..66b09b7c 100644 --- a/ocw/tables.py +++ b/ocw/tables.py @@ -1,13 +1,13 @@ # tutorial/tables.py +from django_tables2.utils import A +from django.utils.html import format_html +from django.templatetags.static import static +from django.template.loader import get_template import django_tables2 as tables import django_filters from .models import Instance from .models import ProviderChoice from .models import StateChoice -from django_tables2.utils import A -from django.utils.html import format_html -from django.templatetags.static import static -from django.template.loader import get_template class NoHeaderLinkColumn(tables.LinkColumn): @@ -40,8 +40,7 @@ def render(self, value, record, bound_column): if value: return format_html('Email notification was send', static('img/notified.png')) - else: - return "" + return "" class TagsColumn(tables.TemplateColumn): @@ -50,7 +49,7 @@ def __init__(self, template_name=None, **extra): super().__init__(template_name="ocw/tags.html", orderable=False, **extra) @property - def header(self, **kwargs): + def header(self): return get_template('ocw/tags_header.html').render() @@ -78,12 +77,12 @@ def render_age(self, record): def render_ttl(self, record): return record.ttl_formated() - class Meta: + class Meta: # pylint: disable=too-few-public-methods model = Instance exclude = ['active'] template_name = 'django_tables2/bootstrap.html' row_attrs = { - 'class': lambda record: "state_{}".format(record.state) + 'class': lambda record: f"state_{record.state}" } @@ -92,14 +91,14 @@ class BaseFilterSet(django_filters.FilterSet): def __init__(self, data=None, *args, **kwargs): if data is not None: data = data.copy() - for name, f in self.base_filters.items(): - initial = f.extra.get('initial') + for name, filter_ in self.base_filters.items(): + initial = filter_.extra.get('initial') if not data.get(name) and initial is not None: if isinstance(initial, list): data.setlistdefault(name, initial) else: data.setdefault(name, initial) - super(BaseFilterSet, self).__init__(data, *args, **kwargs) + super().__init__(data, *args, **kwargs) class InstanceFilter(BaseFilterSet): @@ -110,6 +109,6 @@ class InstanceFilter(BaseFilterSet): instance_id = django_filters.CharFilter(lookup_expr='icontains', field_name='instance_id') ignore = django_filters.BooleanFilter(field_name='ignore', initial=False) - class Meta: + class Meta: # pylint: disable=too-few-public-methods model = Instance fields = ['provider', 'state', 'instance_id', 'region', 'ignore'] diff --git a/ocw/views.py b/ocw/views.py index 4422828b..00066aad 100644 --- a/ocw/views.py +++ b/ocw/views.py @@ -1,30 +1,33 @@ +from django.contrib.auth.decorators import login_required +from django.http import JsonResponse, HttpResponse +from django.core.serializers import serialize from django.shortcuts import redirect from django_tables2 import SingleTableView from .lib import db from .models import Instance from .tables import InstanceTable from .tables import InstanceFilter -from django.contrib.auth.decorators import login_required -from django.http import JsonResponse, HttpResponse -from django.core.serializers import serialize + +# pylint: disable=unused-argument -class FilteredSingleTableView(SingleTableView): +class FilteredSingleTableView(SingleTableView): # pylint: disable=too-many-ancestors filter_class = None + filter = None def get_table_data(self): - data = super(FilteredSingleTableView, self).get_table_data() + data = super().get_table_data() self.filter = self.filter_class(self.request.GET, queryset=data) return self.filter.qs def get_context_data(self, **kwargs): - context = super(FilteredSingleTableView, self).get_context_data(**kwargs) + context = super().get_context_data(**kwargs) context['filter'] = self.filter return context # Displayed with '/ocw/instances' @see urls.py -class FilteredInstanceTableView(FilteredSingleTableView): +class FilteredInstanceTableView(FilteredSingleTableView): # pylint: disable=too-many-ancestors model = Instance table_class = InstanceTable filter_class = InstanceFilter @@ -71,6 +74,6 @@ def update_status(request): @login_required def delete(request, key_id=None): - o = Instance.objects.get(id=key_id) - db.delete_instance(o) + obj = Instance.objects.get(id=key_id) + db.delete_instance(obj) return redirect('update') diff --git a/pylintrc b/pylintrc index 6ae791c6..7c0c7cf0 100644 --- a/pylintrc +++ b/pylintrc @@ -6,7 +6,6 @@ disable= C0206, # Consider iterating with .items() C0415, # Import outside toplevel E1101, # no-member - R0904, # Too many public methods R1702, # Too many nested blocks W0237, # arguments-renamed W0603, # Using the global statement diff --git a/tests/kubernetes.py b/tests/kubernetes.py index d91c07a2..f6e3da21 100644 --- a/tests/kubernetes.py +++ b/tests/kubernetes.py @@ -7,7 +7,9 @@ def load_kube_config(self, *args, **kwargs): class MockedKubernetesClient(): - def __init__(self, jobs=[]): + def __init__(self, jobs=None): + if jobs is None: + jobs = [] self.jobs = jobs self.deleted_jobs = [] diff --git a/tests/test_azure.py b/tests/test_azure.py index 11c6b1ba..3b73716c 100644 --- a/tests/test_azure.py +++ b/tests/test_azure.py @@ -72,7 +72,9 @@ def __init__(self, managed_by=None): class FakeBlobContainer: - def __init__(self, metadata=[], name=None): + def __init__(self, metadata=None, name=None): + if metadata is None: + metadata = [] if name is None: self.name = "sle-images" else: diff --git a/tests/test_gce.py b/tests/test_gce.py index f26aa823..53488e2c 100644 --- a/tests/test_gce.py +++ b/tests/test_gce.py @@ -8,7 +8,9 @@ class MockRequest: - def __init__(self, response={}): + def __init__(self, response=None): + if response is None: + response = {} self.response = response def execute(self): diff --git a/webui/PCWConfig.py b/webui/PCWConfig.py index 8b3fc7b5..d51ab3e9 100644 --- a/webui/PCWConfig.py +++ b/webui/PCWConfig.py @@ -44,7 +44,9 @@ def get(self, config_path: str, default=None): return default return config_pointer - def getList(self, config_path: str, default: list = []) -> list: + def getList(self, config_path: str, default: list = None) -> list: + if default is None: + default = [] return [i.strip() for i in self.get(config_path, ','.join(default)).split(',')] @@ -83,7 +85,7 @@ def get_feature_property(feature: str, property: str, namespace: str = None): def get_namespaces_for(feature: str) -> list: if PCWConfig.has(feature): return ConfigFile().getList(f'{feature}/namespaces', ConfigFile().getList('default/namespaces')) - return list() + return [] @staticmethod def get_providers_for(feature: str, namespace: str): @@ -114,7 +116,7 @@ def has(setting: str) -> bool: @staticmethod def getBoolean(config_path: str, namespace: str = None, default=False) -> bool: if namespace: - (feature, property) = config_path.split('/') + feature, property = config_path.split('/') setting = f'{feature}.namespace.{namespace}/{property}' if PCWConfig.has(setting): value = ConfigFile().get(setting) diff --git a/webui/settings.py b/webui/settings.py index 2ecdfa5d..6bd94ca5 100644 --- a/webui/settings.py +++ b/webui/settings.py @@ -1,4 +1,3 @@ -import re import os import logging.config from django.core.management import utils @@ -27,7 +26,7 @@ SECRET_KEY = utils.get_random_secret_key() # SECURITY WARNING: don't run with debug turned on in production! -DEBUG = os.getenv('PCW_DEBUG', False) +DEBUG = os.getenv('PCW_DEBUG') ALLOWED_HOSTS = ['*'] @@ -187,8 +186,7 @@ def build_absolute_uri(path=''): if not base_url.startswith("http"): base_url = f'https://{base_url}' - - base_url = re.sub('/+$', '', base_url) + base_url = base_url.rstrip("/") if len(path) == 0: return base_url From 370c0ee6f2c2aab750eea22c26b5a8385d5d5761 Mon Sep 17 00:00:00 2001 From: Ricardo Branco Date: Mon, 3 Jul 2023 13:43:20 +0200 Subject: [PATCH 2/2] Fix invalid-overridden-method --- ocw/tables.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ocw/tables.py b/ocw/tables.py index 66b09b7c..554a52e7 100644 --- a/ocw/tables.py +++ b/ocw/tables.py @@ -11,6 +11,7 @@ class NoHeaderLinkColumn(tables.LinkColumn): + @property def header(self): return "" @@ -20,6 +21,7 @@ def __init__(self, *args, **kwargs): kwargs['accessor'] = 'pk' super().__init__(*args, **kwargs) + @property def header(self): return "" @@ -32,6 +34,7 @@ def render(self, record): class MailColumn(tables.BooleanColumn): + @property def header(self): return ""