From c3abe828fcf9d0b899bf3a2f20d2abb0348fdb5b Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Tue, 19 Jul 2022 22:35:49 +0000 Subject: [PATCH] cleanup --- .devcontainer/devcontainer.json | 12 ++- .devcontainer/requirements-dev.txt | 1 + netbox_access_lists/forms/bulk_edit.py | 19 ++-- netbox_access_lists/forms/filtersets.py | 22 ++--- netbox_access_lists/forms/models.py | 118 ++++++------------------ pyproject.toml => pyproject.old | 26 ++++++ 6 files changed, 75 insertions(+), 123 deletions(-) rename pyproject.toml => pyproject.old (56%) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 00b3d59..536b56a 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -27,15 +27,17 @@ //"python.sortImports.args": [ // "--profile=black" //], - //"python.sortImports.path": "/opt/netbox/venv/bin/isort", + "python.sortImports.path": "/opt/netbox/venv/bin/isort", "python.analysis.typeCheckingMode": "strict", "python.analysis.extraPaths": [ "/opt/netbox/", "/opt/netbox/netbox" ], "python.autoComplete.extraPaths": [ - "/opt/netbox/", - "/opt/netbox/netbox" + "/opt/netbox/netbox/", + "/opt/netbox/netbox/**", + "/opt/netbox/netbox/**/**" + ], "python.defaultInterpreterPath": "/opt/netbox/venv/bin/python3", "python.formatting.autopep8Path": "/opt/netbox/venv/bin/autopep8", @@ -49,7 +51,9 @@ "python.linting.pycodestylePath": "/opt/netbox/venv/bin/pycodestyle", "python.linting.pydocstylePath": "/opt/netbox/venv/bin/pydocstyle", "python.linting.pylintArgs": [ - "--enable=W0614" + "--django-settings-module=/opt/netbox/netbox/netbox/netbox.settings", + "--enable=W0602,W0611,W0612,W0613,W0614", + "--load-plugins=pylint_django" ], "python.linting.pylintEnabled": true, "python.linting.pylintPath": "/opt/netbox/venv/bin/pylint", diff --git a/.devcontainer/requirements-dev.txt b/.devcontainer/requirements-dev.txt index 9f60b96..093cf52 100644 --- a/.devcontainer/requirements-dev.txt +++ b/.devcontainer/requirements-dev.txt @@ -8,4 +8,5 @@ pre-commit pycodestyle pydocstyle pylint +pylint-django yapf diff --git a/netbox_access_lists/forms/bulk_edit.py b/netbox_access_lists/forms/bulk_edit.py index eb3079e..1a3d1e2 100644 --- a/netbox_access_lists/forms/bulk_edit.py +++ b/netbox_access_lists/forms/bulk_edit.py @@ -2,20 +2,13 @@ from dcim.models import Device, Region, Site, SiteGroup from django import forms from django.core.exceptions import ValidationError from django.utils.safestring import mark_safe -from extras.models import Tag -from ipam.models import Prefix -from netbox.forms import (NetBoxModelBulkEditForm, NetBoxModelFilterSetForm, - NetBoxModelForm) -from utilities.forms import (ChoiceField, CommentField, - DynamicModelChoiceField, - DynamicModelMultipleChoiceField, - MultipleChoiceField, StaticSelect, - StaticSelectMultiple, TagFilterField, - add_blank_choice) +from netbox.forms import NetBoxModelBulkEditForm +from utilities.forms import (ChoiceField, + DynamicModelChoiceField, StaticSelect, + add_blank_choice) -from netbox_access_lists.models import (AccessList, ACLActionChoices, ACLExtendedRule, - ACLProtocolChoices, ACLRuleActionChoices, ACLStandardRule, - ACLTypeChoices) +from netbox_access_lists.models import (AccessList, ACLActionChoices, + ACLTypeChoices) #__all__ = ( # 'AccessListBulkEditForm', diff --git a/netbox_access_lists/forms/filtersets.py b/netbox_access_lists/forms/filtersets.py index 606bff7..e33f7ef 100644 --- a/netbox_access_lists/forms/filtersets.py +++ b/netbox_access_lists/forms/filtersets.py @@ -1,21 +1,15 @@ from dcim.models import Device, Region, Site, SiteGroup from django import forms -from django.core.exceptions import ValidationError -from django.utils.safestring import mark_safe -from extras.models import Tag from ipam.models import Prefix -from netbox.forms import (NetBoxModelBulkEditForm, NetBoxModelFilterSetForm, - NetBoxModelForm) -from utilities.forms import (ChoiceField, CommentField, - DynamicModelChoiceField, - DynamicModelMultipleChoiceField, - MultipleChoiceField, StaticSelect, - StaticSelectMultiple, TagFilterField, - add_blank_choice) +from netbox.forms import NetBoxModelFilterSetForm +from utilities.forms import (ChoiceField, DynamicModelChoiceField, + StaticSelect, StaticSelectMultiple, + TagFilterField, add_blank_choice) -from netbox_access_lists.models import (AccessList, ACLActionChoices, ACLExtendedRule, - ACLProtocolChoices, ACLRuleActionChoices, ACLStandardRule, - ACLTypeChoices) +from netbox_access_lists.models import (AccessList, ACLActionChoices, + ACLExtendedRule, ACLProtocolChoices, + ACLRuleActionChoices, ACLStandardRule, + ACLTypeChoices) __all__ = ( 'AccessListFilterForm', diff --git a/netbox_access_lists/forms/models.py b/netbox_access_lists/forms/models.py index 0c48029..615d806 100644 --- a/netbox_access_lists/forms/models.py +++ b/netbox_access_lists/forms/models.py @@ -1,21 +1,12 @@ from dcim.models import Device, Region, Site, SiteGroup from django import forms -from django.core.exceptions import ValidationError from django.utils.safestring import mark_safe from extras.models import Tag from ipam.models import Prefix -from netbox.forms import (NetBoxModelBulkEditForm, NetBoxModelFilterSetForm, - NetBoxModelForm) -from utilities.forms import (ChoiceField, CommentField, - DynamicModelChoiceField, - DynamicModelMultipleChoiceField, - MultipleChoiceField, StaticSelect, - StaticSelectMultiple, TagFilterField, - add_blank_choice) +from netbox.forms import NetBoxModelForm +from utilities.forms import (CommentField, DynamicModelChoiceField, DynamicModelMultipleChoiceField) -from netbox_access_lists.models import (AccessList, ACLActionChoices, ACLExtendedRule, - ACLProtocolChoices, ACLRuleActionChoices, ACLStandardRule, - ACLTypeChoices) +from netbox_access_lists.models import (AccessList, ACLExtendedRule, ACLStandardRule) __all__ = ( 'AccessListForm', @@ -76,11 +67,16 @@ class AccessListForm(NetBoxModelForm): device = cleaned_data.get('device') type = cleaned_data.get('type') if ('name' in self.changed_data or 'device' in self.changed_data) and AccessList.objects.filter(name__iexact=name, device=device).exists(): - raise forms.ValidationError('An ACL with this name (case insensitive) is already associated to this device.') + raise forms.ValidationError( + { + 'device': ['An ACL with this name (case insensitive) is already associated to this device.'], + 'name': ['An ACL with this name (case insensitive) is already associated to this device.'], + } + ) if type == 'extended' and self.instance.aclstandardrules.exists(): - raise forms.ValidationError('This ACL has Standard ACL rules already associated, CANNOT change ACL type!!') + raise forms.ValidationError({'type': ['This ACL has Standard ACL rules already associated, CANNOT change ACL type!!']}) elif type == 'standard' and self.instance.aclextendedrules.exists(): - raise forms.ValidationError('This ACL has Extended ACL rules already associated, CANNOT change ACL type!!') + raise forms.ValidationError({'type': ['This ACL has Extended ACL rules already associated, CANNOT change ACL type!!']}) return cleaned_data @@ -124,7 +120,12 @@ class ACLStandardRuleForm(NetBoxModelForm): cleaned_data = super().clean() if cleaned_data.get('action') == 'remark': if cleaned_data.get('remark') is None: - raise forms.ValidationError('Action Remark is set, MUST set a remark') + raise forms.ValidationError( + { + 'action': ['Action Remark is set, MUST add a remark.'], + 'remark': ['Action Remark is set, MUST add a remark.'], + } + ) if cleaned_data.get('source_prefix'): raise forms.ValidationError('Cannot input a remark AND a source prefix. Remove one.') else: @@ -183,6 +184,13 @@ class ACLExtendedRuleForm(NetBoxModelForm): def clean(self): cleaned_data = super().clean() if cleaned_data.get('action') == 'remark': + if cleaned_data.get('remark') is None: + raise forms.ValidationError( + { + 'action': ['Action Remark is set, MUST add a remark.'], + 'remark': ['Action Remark is set, MUST add a remark.'], + } + ) if cleaned_data.get('remark') is None: raise forms.ValidationError('Action Remark is set, MUST set a remark') if cleaned_data.get('source_prefix'): @@ -195,80 +203,6 @@ class ACLExtendedRuleForm(NetBoxModelForm): raise forms.ValidationError('CANNOT set an action of remark AND destination ports.') if cleaned_data.get('protocol'): raise forms.ValidationError('CANNOT set an action of remark AND a protocol.') - else: - if cleaned_data.get('remark'): - raise forms.ValidationError('CANNOT set a remark without the action set to remark also.') - #if cleaned_data.get('access_list_type') == 'standard' and (source_ports or destination_prefix or destination_ports): - # raise forms.ValidationError('Standard Access-Lists only allow a source_prefix or remark') + elif cleaned_data.get('remark'): + raise forms.ValidationError('CANNOT set a remark without the action set to remark also.') return cleaned_data - - -# -# Bulk Edit Forms -# - -#class AccessListBulkEditForm(NetBoxModelBulkEditForm): -# model = AccessList -# -# region = DynamicModelChoiceField( -# queryset=Region.objects.all(), -# required=False, -# ) -# site_group = DynamicModelChoiceField( -# queryset=SiteGroup.objects.all(), -# required=False, -# label='Site Group' -# ) -# site = DynamicModelChoiceField( -# queryset=Site.objects.all(), -# required=False -# ) -# device = DynamicModelChoiceField( -# queryset=Device.objects.all(), -# query_params={ -# 'region': '$region', -# 'group_id': '$site_group', -# 'site_id': '$site', -# }, -# required=False, -# ) -# type = ChoiceField( -# choices=add_blank_choice(ACLTypeChoices), -# required=False, -# widget=StaticSelect(), -# ) -# default_action = ChoiceField( -# choices=add_blank_choice(ACLActionChoices), -# required=False, -# widget=StaticSelect(), -# label='Default Action', -# ) -# -# fieldsets = [ -# ('Host Details', ('region', 'site_group', 'site', 'device')), -# ('Access-List Details', ('type', 'default_action', 'add_tags', 'remove_tags')), -# ] -# -# class Meta: -# model = AccessList -# fields = ('region', 'site_group', 'site', 'device', 'type', 'default_action', 'add_tags', 'remove_tags') -# help_texts = { -# 'default_action': 'The default behavior of the ACL.', -# 'name': 'The name uniqueness per device is case insensitive.', -# 'type': mark_safe('*Note: CANNOT be changed if ACL Rules are assoicated to this Access-List.'), -# } -# -# def clean(self): # Not working given you are bulkd editing multiple forms -# cleaned_data = super().clean() -# if self.errors.get('name'): -# return cleaned_data -# name = cleaned_data.get('name') -# device = cleaned_data.get('device') -# type = cleaned_data.get('type') -# if ('name' in self.changed_data or 'device' in self.changed_data) and AccessList.objects.filter(name__iexact=name, device=device).exists(): -# raise forms.ValidationError('An ACL with this name (case insensitive) is already associated to this device.') -# if type == 'extended' and self.cleaned_data['aclstandardrules'].exists(): -# raise forms.ValidationError('This ACL has Standard ACL rules already associated, CANNOT change ACL type!!') -# elif type == 'standard' and self.cleaned_data['aclextendedrules'].exists(): -# raise forms.ValidationError('This ACL has Extended ACL rules already associated, CANNOT change ACL type!!') -# return cleaned_data diff --git a/pyproject.toml b/pyproject.old similarity index 56% rename from pyproject.toml rename to pyproject.old index d3e69f4..4203794 100644 --- a/pyproject.toml +++ b/pyproject.old @@ -1,3 +1,7 @@ +[MASTER] +load-plugins=pylint_django +django-settings-module=/opt/netbox/netbox/netbox/netbox.settings + [tool.black] line-length = 100 @@ -25,5 +29,27 @@ disable = [ "unsubscriptable-object", ] enable = [ + "expression-not-assigned", + "global-variable-not-assigned", + "possibly-unused-variable", + "reimported", + "unused-argument", + "unused-import", + "unused-variable", + "unused-wildcard-import", + "useless-else-on-loop", + "useless-import-alias", + "useless-object-inheritance", + "useless-parent-delegation", + "useless-return", "useless-suppression", # Identify unneeded pylint disable statements ] + +[tool.isort] +force_grid_wrap = 0 +include_trailing_comma = true +line_length = 79 +multi_line_output = 1 +overwrite_in_place = true +use_parentheses = true +verbose = true