CI: Bump pylint version to 2.16.1 (aristanetworks#587)

* CI: Bump pylint version to 2.16.1

* Added TODO comments

* bump ansible-core version; ansible-test fixes

* bumped ansible-lint

* bump min ansible-core to 2.14.0

* bump ansible-core bugfix

* bump min python to 3.9

* bumped container version

.github/workflows/pull-request-conflict.yml

@@ -6,7 +6,7 @@ on:
 jobs:
   Conflict_Check:
     name: 'Check PR status: conflicts and resolution'
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-20.04
     steps:
       - name: check if PRs are dirty
         uses: eps1lon/actions-label-merge-conflict@releases/2.x

.github/workflows/pull-request-management.yml

@@ -78,8 +78,8 @@ jobs:
           fetch-depth: 0
       - name: Install requirements
         run: |
-          pip install -r ansible_collections/arista/cvp/requirements-dev.txt
-          pip install -r ansible_collections/arista/cvp/requirements.txt
+          pip install -r ansible_collections/arista/cvp/requirements-dev.txt --upgrade
+          pip install -r ansible_collections/arista/cvp/requirements.txt --upgrade
       - name: Run pre-commit
         run: |
           pre-commit run --color always --from-ref origin/devel --to-ref HEAD
@@ -93,7 +93,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        python_version: [ 3.8, 3.9 ]
+        python_version: [ 3.9 ]
     steps:
       - name: 'set environment variables'
         run: |
@@ -119,7 +119,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        python_version: [ 3.8, 3.9 ]
+        python_version: [ 3.9 ]
     steps:
       - name: 'set environment variables'
         run: |
@@ -254,7 +254,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        python_version: [ 3.8, 3.9 ]
+        python_version: [ 3.9 ]
     steps:
       - name: 'set environment variables'
         run: |

.pre-commit-config.yaml

@@ -9,13 +9,16 @@ repos:
       - id: check-added-large-files
       - id: check-merge-conflict
 
-  - repo: https://github.com/pre-commit/mirrors-pylint
-    rev: 'v2.5.0'
+  - repo: https://github.com/pycqa/pylint
+    rev: "v2.16.1"
     hooks:
-    - id: pylint # Use pylintrc file in repository
-      name: Check for Linting error on Python files
-      description: This hook runs pylint.
-      exclude: ^.*(docs|tests).*
+      - id: pylint # Use pylintrc file in repository
+        name: Check for Linting errors on Python files
+        description: This hook runs pylint.
+        types: [python]
+        args:
+          # Suppress duplicate code for modules header
+          - -d duplicate-code
 
   - repo: https://github.com/adrienverge/yamllint.git
     rev: 'v1.23.0'

Makefile

@@ -138,8 +138,8 @@ github-configure-ci-python3: ## Configure Python3 environment to run GA (Ubuntu:
 .PHONY: install-requirements
 install-requirements: ## Install python requirements for generic purpose
 	pip3 install --upgrade wheel pip
-	pip3 install -r ansible_collections/arista/cvp/requirements.txt
-	pip3 install -r ansible_collections/arista/cvp/requirements-dev.txt
+	pip3 install -r ansible_collections/arista/cvp/requirements.txt --upgrade
+	pip3 install -r ansible_collections/arista/cvp/requirements-dev.txt --upgrade
 
 .PHONY: install-docker
 install-docker: ## Install docker

ansible_collections/arista/cvp/docs/_build/ansible2rst.py

@@ -31,7 +31,7 @@
 from ansible.parsing.yaml.loader import AnsibleLoader
 from ansible.plugins.loader import fragment_loader
 from ansible.module_utils._text import to_bytes
-from ansible.errors import AnsibleError, AnsibleParserError
+from ansible.errors import AnsibleError
 
 try:
     from html import escape as html_escape
@@ -44,6 +44,9 @@ def html_escape(text, quote=True):
 
 from ansible import __version__ as ansible_version
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 #####################################################################################
 # constants and paths
 
@@ -74,7 +77,7 @@ def too_old(added):
         added_tokens = str(added).split(".")
         readded = added_tokens[0] + "." + added_tokens[1]
         added_float = float(readded)
-    except ValueError as e:
+    except ValueError:
         return False
     return added_float < TO_OLD_TO_BE_NOTABLE
 
@@ -173,7 +176,7 @@ def jinja2_environment(template_dir, template_type):
         template = env.get_template('rst.j2')
         outputname = "%s.rst"
     else:
-        raise Exception("unknown module format type: %s" % template_type)
+        raise TypeError(f"unknown module format type: {template_type}")
 
     return env, template, outputname
 
@@ -213,8 +216,7 @@ def add_fragments(doc, filename):
                 doc['notes'].extend(notes)
 
         if 'options' not in fragment and 'logging_options' not in fragment and 'connection_options' not in fragment:
-            raise Exception("missing options in fragment (%s), possibly misformatted?: %s" % (
-                fragment_name, filename))
+            raise ValueError(f"missing options in fragment ({fragment_name}), possibly misformatted?: {filename}")
 
         for key, value in iteritems(fragment):
             if key in doc:
@@ -226,8 +228,7 @@ def add_fragments(doc, filename):
                 elif isinstance(doc[key], MutableSequence):
                     value = sorted(frozenset(value + doc[key]))
                 else:
-                    raise Exception("Attempt to extend a documentation fragement (%s) of unknown type: %s" % (
-                        fragment_name, filename))
+                    raise TypeError(f"Attempt to extend a documentation fragement ({fragment_name}) of unknown type: {filename}")
             doc[key] = value
 
 
@@ -419,33 +420,33 @@ def main():
             module_names.append(module.replace(".py", ""))
 
     index_file_path = os.path.join(OUTPUTDIR, "index.rst")
-    index_file = open(index_file_path, "w")
-    index_file.write('Arista Cloudvision Ansible Modules\n')
-    index_file.write('==================================\n')
-    index_file.write('\n')
-    index_file.write('.. toctree::\n')
-    index_file.write('  :caption: Project Documentation\n')
-    index_file.write('  :name: mastertoc::\n')
-    index_file.write('  :maxdepth: 1\n')
-    index_file.write('\n')
-    index_file.write('   Collection Overview <../README.md>\n')
-    index_file.write('\n')
-    index_file.write('.. toctree::\n')
-    index_file.write('  :caption: Roles List\n')
-    index_file.write('  :name: roletoc::\n')
-    index_file.write('  :maxdepth: 1\n')
-    index_file.write('\n')
-    index_file.write(
-        '   DHCP Configuration <../ansible_collections/arista/cvp/roles/dhcp_configuration/README.md>\n')
-    index_file.write('\n')
-    index_file.write('.. toctree::\n')
-    index_file.write('  :caption: Modules List\n')
-    index_file.write('  :name: moduletoc::\n')
-    index_file.write('  :maxdepth: 1\n')
-    index_file.write('\n')
-
-    for module_name in module_names:
-        index_file.write('   Module arista.cvp.%s <%s>\n' % (module_name, module_name))
+    with open(index_file_path, "w", encoding="UTF-8") as index_file:
+        index_file.write('Arista Cloudvision Ansible Modules\n')
+        index_file.write('==================================\n')
+        index_file.write('\n')
+        index_file.write('.. toctree::\n')
+        index_file.write('  :caption: Project Documentation\n')
+        index_file.write('  :name: mastertoc::\n')
+        index_file.write('  :maxdepth: 1\n')
+        index_file.write('\n')
+        index_file.write('   Collection Overview <../README.md>\n')
+        index_file.write('\n')
+        index_file.write('.. toctree::\n')
+        index_file.write('  :caption: Roles List\n')
+        index_file.write('  :name: roletoc::\n')
+        index_file.write('  :maxdepth: 1\n')
+        index_file.write('\n')
+        index_file.write(
+            '   DHCP Configuration <../ansible_collections/arista/cvp/roles/dhcp_configuration/README.md>\n')
+        index_file.write('\n')
+        index_file.write('.. toctree::\n')
+        index_file.write('  :caption: Modules List\n')
+        index_file.write('  :name: moduletoc::\n')
+        index_file.write('  :maxdepth: 1\n')
+        index_file.write('\n')
+
+        for module_name in module_names:
+            index_file.write('   Module arista.cvp.%s <%s>\n' % (module_name, module_name))
 
 
 if __name__ == '__main__':

ansible_collections/arista/cvp/docs/installation/requirements.md

@@ -19,11 +19,11 @@
 
 ## Python
 
-- Python **3.8** or later
+- Python **3.9** or later
 
 ## Supported Ansible Versions
 
-- ansible-core from **2.11.3** to **2.12.x**
+- ansible-core from **2.12.6** to **2.15.0** excluding **2.13.0**
 
 ## Additional Python Libraries required
 

ansible_collections/arista/cvp/molecule/cv_configlet_strict/converge.yml

@@ -21,7 +21,7 @@
       arista.cvp.cv_configlet:
         cvp_facts: "{{ansible_facts}}"
         configlets: "{{CVP_CONFIGLETS}}"
-        configlet_filter: ["TRAINING","01TRAINING-01","01TRAINING-02"]
+        configlet_filter: ["TRAINING", "01TRAINING-01", "01TRAINING-02"]
         state: present
         filter_mode: strict
       register: CVP_CONFIGLET_RESULT

ansible_collections/arista/cvp/plugins/module_utils/change_tools.py

@@ -40,6 +40,9 @@
 MODULE_LOGGER = logging.getLogger('arista.cvp.change_tools')
 MODULE_LOGGER.info('Start change_tools module execution')
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 
 class CvpChangeControlBuilder:
     """
@@ -270,7 +273,7 @@ def __genID__(self):
             id = str(uuid.uuid4())
             if id not in self.__keyStore:
                 self.__keyStore.append(id)
-                return(id)
+                return (id)
             else:
                 # Keep looping until we get a unique ID
                 pass
@@ -599,8 +602,8 @@ def get_change_control(self, cc_id):
     def module_action(self, change: dict, name: str = None, state: str = "show", change_id: List[str] = None, schedule_time: str = None):
 
         changed = False
-        data = dict()
-        warnings = list()
+        data = {}
+        warnings = []
 
         if state == "show":
             if name is None and change_id is None:

ansible_collections/arista/cvp/plugins/module_utils/container_tools.py

@@ -24,7 +24,6 @@
 import pprint
 from typing import List
 from ansible.module_utils.basic import AnsibleModule
-import ansible_collections.arista.cvp.plugins.module_utils.logger  # noqa # pylint: disable=unused-import
 from ansible_collections.arista.cvp.plugins.module_utils.resources.api.fields import Api
 from ansible_collections.arista.cvp.plugins.module_utils.resources.modules.fields import ContainerResponseFields, ModuleOptionValues
 from ansible_collections.arista.cvp.plugins.module_utils.response import CvApiResult, CvManagerResult, CvAnsibleResponse
@@ -43,6 +42,10 @@
 MODULE_LOGGER.info('Start cv_container_v3 module execution')
 
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
+
 class ContainerInput(object):
     """
     ContainerInput Object to manage Container Topology in context of arista.cvp collection.
@@ -479,6 +482,8 @@ def __image_bundle_add(self, container: dict, image_bundle: str):
                     MODULE_LOGGER.info("Checking if container has an image bundle already")
                     current_image_facts = self.__cvp_client.api.get_image_bundle_by_container_id(container[Api.generic.KEY])
                     pass
+                # TODO - remove variable e
+                # pylint: disable=unused-variable
                 except CvpApiError as e:
                     message = "Error retrieving image bundle info for container: " + str(container[Api.generic.KEY])
                     MODULE_LOGGER.error(message)

ansible_collections/arista/cvp/plugins/module_utils/device_tools.py

@@ -49,6 +49,9 @@
 MODULE_LOGGER = logging.getLogger(__name__)
 MODULE_LOGGER.info('Start device_tools module execution')
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 
 class DeviceElement(object):
     """
@@ -793,6 +796,9 @@ def __get_configlet_list_inherited_from_container(self, device: DeviceElement):
         parent_container_name = device.container
         # Get parent container id of the device by comparing all containers name in the topology
         parent_container_id = ''
+        # TODO - use dict items
+        #      for container_id in ..
+        # pylint: disable=consider-using-dict-items
         for container_id in self.__containers_configlet_list_cache:
             if self.__containers_configlet_list_cache[container_id][Api.generic.NAME] == parent_container_name:
                 parent_container_id = container_id

ansible_collections/arista/cvp/plugins/module_utils/image_tools.py

@@ -38,6 +38,9 @@
 MODULE_LOGGER = logging.getLogger('arista.cvp.image_tools')
 MODULE_LOGGER.info('Start image_tools module execution')
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 
 class CvImageTools():
     """

ansible_collections/arista/cvp/plugins/module_utils/response.py

@@ -261,7 +261,7 @@ def results(self):
         dict
             Dictionary with all values
         """
-        result = dict()
+        result = {}
         result[FIELD_SUCCESS] = self.__success
         result[FIELD_CHANGED] = self.__changed
         result[FIELD_TASKIDS] = self.__taskIds
@@ -304,10 +304,10 @@ def __init__(self, builder_name: str, default_success: bool = False):
         self.__success = default_success
         self.__changed = False
         self.__counter: int = 0
-        self.__taskIds = list()
-        self.__changes = dict()
-        self.__diffs = dict()
-        self.__changes[self.__name + FIELD_CHANGE_LIST] = list()
+        self.__taskIds = []
+        self.__changes = {}
+        self.__diffs = {}
+        self.__changes[self.__name + FIELD_CHANGE_LIST] = []
 
     def add_change(self, change: CvApiResult):
         """

ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py

@@ -168,7 +168,7 @@ def match_filter(input, filter, default_always='all'):
 
 
 def cv_update_configlets_on_device(module, device_facts, add_configlets, del_configlets):
-    response = dict()
+    response = {}
     device_deletion = None
     device_addition = None
     # Initial Logging

ansible_collections/arista/cvp/plugins/modules/cv_change_control_v3.py

@@ -164,6 +164,9 @@ def main():
     """
     main entry point for module execution.
     """
+    # TODO - ansible module prefers constructor over literal
+    #        for dict
+    # pylint: disable=use-dict-literal
     argument_spec = dict(
         name=dict(type='str'),
         change=dict(type='dict'),

ansible_collections/arista/cvp/plugins/modules/cv_configlet.py

@@ -132,6 +132,9 @@
 MODULE_LOGGER = logging.getLogger('arista.cvp.cv_configlet')
 MODULE_LOGGER.info('Start cv_configlet module execution')
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 
 def get_tasks(taskIds, module):
     taskList = list()

ansible_collections/arista/cvp/plugins/modules/cv_configlet_v3.py

@@ -99,6 +99,9 @@
 MODULE_LOGGER = logging.getLogger('arista.cvp.cv_configlet')
 MODULE_LOGGER.info('Start cv_configlet module execution')
 
+# TODO - use f-strings
+# pylint: disable=consider-using-f-string
+
 
 def check_import(ansible_module: AnsibleModule):
     """
@@ -129,6 +132,9 @@ def main():
     """
     Main entry point for module execution.
     """
+    # TODO - ansible module prefers constructor over literal
+    #        for dict
+    # pylint: disable=use-dict-literal
     argument_spec = dict(
         # Topology to configure on CV side.
         configlets=dict(type='dict', required=True),
@@ -146,8 +152,8 @@ def main():
                                    supports_check_mode=True)
     # Instantiate ansible results
     result = dict(changed=False, data={}, failed=False)
-    result['data']['taskIds'] = list()
-    result['data']['tasks'] = list()
+    result['data']['taskIds'] = []
+    result['data']['tasks'] = []
 
     # State management
     is_present = True