Merge pull request #277 from ricardobranco777/fixes

Minor fixes

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:

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):

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):

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))
 

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'),

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'),)
 
 

ocw/tables.py

@@ -1,16 +1,17 @@
 # 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):
+    @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 ""
 
@@ -40,8 +43,7 @@ def render(self, value, record, bound_column):
         if value:
             return format_html('<img alt="Email notification was send" src="{}" width=20 height=20/>',
                                static('img/notified.png'))
-        else:
-            return ""
+        return ""
 
 
 class TagsColumn(tables.TemplateColumn):
@@ -50,7 +52,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 +80,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 +94,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 +112,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']

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')

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

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 = []
 

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:

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):

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)

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