Add Python linter to CircleCI and pre-commit (DataDog#5712)

GetTerminus · Jun 16, 2020 · 1a0dbc1 · 1a0dbc1
1 parent 901e157
commit 1a0dbc1
Show file tree

Hide file tree

Showing 46 changed files with 123 additions and 142 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -63,6 +63,7 @@ jobs:
           command: |
             pip install wheel
             pip install -r requirements.txt
+            pip3 install flake8==3.8.3 flake8-bugbear==20.1.4
       - run:
           name: grab go deps
           command: |
@@ -93,6 +94,7 @@ jobs:
             - /go/pkg
             - /go/bin
             - /usr/local/lib/python2.7/dist-packages
+            - /usr/local/lib/python3.6/dist-packages
             - /usr/local/bin
 
   unit_tests:
@@ -173,6 +175,15 @@ jobs:
           #SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
           command: shellcheck --severity=info -e SC2059 -e SC2028 --shell=bash ./cmd/**/*.sh ./omnibus/package-scripts/*/*
 
+  python_linting:
+    <<: *job_template
+    steps:
+      - restore_cache: *restore_source
+      - restore_cache: *restore_deps
+      - run:
+          name: lint python files
+          command: inv -e lint-python
+
   docker_tests:
     <<: *job_template
     steps:
@@ -298,6 +309,9 @@ workflows:
       - shell_linting:
           requires:
             - dependencies
+      - python_linting:
+          requires:
+            - dependencies
       - docker_tests:
           requires:
             - dependencies

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -2,8 +2,14 @@ default_language_version:
   python: 3.8.3
 
 repos:
+- repo: https://gitlab.com/pycqa/flake8
+  rev: 3.8.3 # should match Python version
+  hooks:
+    - id: flake8
+      additional_dependencies:
+        - flake8-bugbear==20.1.4
 - repo: https://github.com/jumanjihouse/pre-commit-hooks
   rev: 2.1.1
   hooks:
     - id: shellcheck
-      args: ["--severity=info", "-e", "SC2059", "-e", "SC2028"]
+      args: ["--severity=info", "-e", "SC2059", "-e", "SC2028"]
diff --git a/Dockerfiles/agent/secrets-helper/test_readsecret.py b/Dockerfiles/agent/secrets-helper/test_readsecret.py
@@ -4,14 +4,15 @@
 import shutil
 import tempfile
 import unittest
+import os
 
-from readsecret import *
+from readsecret import list_secret_names, read_file, is_valid_folder
 
 
 class TestListSecretNames(unittest.TestCase):
     def test_invalid_output(self):
         with self.assertRaisesRegex(
-            ValueError, "Expecting value: line 1 column 1 \(char 0\)"
+            ValueError, r"Expecting value: line 1 column 1 \(char 0\)"
         ):
             list_secret_names("")
 
@@ -71,7 +72,7 @@ def test_file_ok(self):
     def test_file_size_limit(self):
         filename = "big_file"
         with open(os.path.join(self.folder, filename), "w") as f:
-            for i in range(0, 2048):
+            for _ in range(0, 2048):
                 f.write("big")
         contents = read_file(self.folder, filename)
         self.assertEqual(len(contents), 1024)

diff --git a/Dockerfiles/agent/test_image_contents.py b/Dockerfiles/agent/test_image_contents.py
@@ -49,7 +49,7 @@ def test_files_permissions(self):
         def has_write_permissions(path):
             try:
                 return bool(os.stat(path).st_mode & stat.S_IWOTH)
-            except:
+            except Exception:
                 return False
 
         for root, dirs, files in os.walk("/etc"):
@@ -64,7 +64,7 @@ def correct_permissions(root):
     def correct_perm(path):
         try:
             mode = os.stat(path).st_mode
-        except:
+        except Exception:
             return
 
         if bool(mode & stat.S_IWOTH):

diff --git a/cmd/agent/dist/checks/__init__.py b/cmd/agent/dist/checks/__init__.py
@@ -8,5 +8,5 @@
 # As long as we do not properly deprecate the legacy import path, they
 # should not be removed. For more details, see #4032 and #4421.
 
-from datadog_checks.base.checks import AgentCheck
-from datadog_checks.base.errors import CheckException
+from datadog_checks.base.checks import AgentCheck  # noqa: F401
+from datadog_checks.base.errors import CheckException  # noqa: F401
diff --git a/cmd/agent/dist/checks/libs/wmi/sampler.py b/cmd/agent/dist/checks/libs/wmi/sampler.py
@@ -24,4 +24,4 @@
 Original discussion thread: https://github.com/DataDog/dd-agent/issues/1952
 Credits to @TheCloudlessSky (https://github.com/TheCloudlessSky)
 """
-from datadog_checks.base.checks.win.wmi.sampler import WMISampler
+from datadog_checks.base.checks.win.wmi.sampler import WMISampler  # noqa: F401
diff --git a/cmd/agent/dist/checks/network_checks.py b/cmd/agent/dist/checks/network_checks.py
@@ -2,4 +2,4 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.checks import NetworkCheck, Status, EventType
+from datadog_checks.base.checks import NetworkCheck, Status, EventType  # noqa: F401
diff --git a/cmd/agent/dist/checks/prometheus_check/__init__.py b/cmd/agent/dist/checks/prometheus_check/__init__.py
@@ -1,6 +1,6 @@
 # (C) Datadog, Inc. 2016
 # All rights reserved
 # Licensed under Simplified BSD License (see LICENSE)
-from datadog_checks.base.checks.prometheus import (
+from datadog_checks.base.checks.prometheus import (  # noqa: F401
     PrometheusCheck, PrometheusFormat, UnknownFormatError
 )
diff --git a/cmd/agent/dist/checks/winwmi_check.py b/cmd/agent/dist/checks/winwmi_check.py
@@ -2,6 +2,6 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.checks.win.wmi import (
+from datadog_checks.base.checks.win.wmi import ( # noqa: F401
     WinWMICheck, WMIMetric, MissingTagBy, from_time, to_time
 )
diff --git a/cmd/agent/dist/config.py b/cmd/agent/dist/config.py
@@ -2,5 +2,5 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.config import _is_affirmative
-from datadog_checks.log import _get_py_loglevel
+from datadog_checks.config import _is_affirmative  # noqa: F401
+from datadog_checks.log import _get_py_loglevel  # noqa: F401
diff --git a/cmd/agent/dist/utils/containers.py b/cmd/agent/dist/utils/containers.py
@@ -2,4 +2,4 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.utils.containers import hash_mutable
+from datadog_checks.base.utils.containers import hash_mutable  # noqa: F401
diff --git a/cmd/agent/dist/utils/platform.py b/cmd/agent/dist/utils/platform.py
@@ -2,4 +2,4 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.utils.platform import Platform
+from datadog_checks.base.utils.platform import Platform # noqa: F401
diff --git a/cmd/agent/dist/utils/subprocess_output.py b/cmd/agent/dist/utils/subprocess_output.py
@@ -3,4 +3,4 @@
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
 
-from datadog_checks.base.utils.subprocess_output import get_subprocess_output
+from datadog_checks.base.utils.subprocess_output import get_subprocess_output  # noqa: F401
diff --git a/cmd/agent/dist/utils/tailfile.py b/cmd/agent/dist/utils/tailfile.py
@@ -2,4 +2,4 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.utils.tailfile import TailFile
+from datadog_checks.base.utils.tailfile import TailFile # noqa: F401
diff --git a/cmd/agent/dist/utils/timeout.py b/cmd/agent/dist/utils/timeout.py
@@ -2,4 +2,4 @@
 # under the Apache License Version 2.0.
 # This product includes software developed at Datadog (https://www.datadoghq.com/).
 # Copyright 2016-2020 Datadog, Inc.
-from datadog_checks.base.utils.timeout import TimeoutException, timeout
+from datadog_checks.base.utils.timeout import TimeoutException, timeout  # noqa: F401
diff --git a/devenv/tasks/packer.py b/devenv/tasks/packer.py
@@ -2,9 +2,7 @@
 Packer namespaced tasks
 """
 from __future__ import print_function
-import os
 
-import invoke
 from invoke import task
 from invoke.exceptions import Exit
 
@@ -18,12 +16,12 @@
 def build(ctx, os = "windows-10", provider = "virtualbox-iso"):
     """
     Build the Vagrant box
-    
+
     Example invokation:
         inv packer.build --os=windows-10
     """
 
-    if not provider in DEFAULT_BUILDERS:
+    if provider not in DEFAULT_BUILDERS:
         print("Error: unknown provider")
         return Exit(code=1)
 

diff --git a/pkg/collector/runner/runner_test.py b/pkg/collector/runner/runner_test.py
@@ -1,8 +1,8 @@
 # Python module for simulating a simple python check
 
 from checks import AgentCheck
-import time;
-from time import sleep;
+import time
+from time import sleep
 
 class TestCheck(AgentCheck):
     def check(self, instance):

diff --git a/pkg/config/legacy/tests/config.py b/pkg/config/legacy/tests/config.py
@@ -6,22 +6,16 @@
 # stdlib
 import ConfigParser
 from cStringIO import StringIO
-import glob
-import imp
-import inspect
-import itertools
 import logging
 import logging.config
 import logging.handlers
-from optparse import OptionParser, Values
 import os
-import platform
 import re
 from socket import gaierror, gethostbyname
 import string
 import sys
-import traceback
 from urlparse import urlparse
+from urllib.request import getproxies
 
 # 3p
 import json
@@ -44,6 +38,7 @@
 SD_PIPE_NAME = "dd-service_discovery"
 SD_PIPE_UNIX_PATH = '/opt/datadog-agent/run'
 SD_PIPE_WIN_PATH = "\\\\.\\pipe\\{pipename}"
+SD_TEMPLATE_DIR = "/datadog/check_configs"
 
 log = logging.getLogger(__name__)
 
@@ -358,7 +353,7 @@ def get_config(cfg_path=None, options=None, can_query_registry=True):
             try:
                 additional_config = extract_agent_config(config)
                 agentConfig.update(additional_config)
-            except:
+            except Exception:
                 log.error('Failed to load the agent configuration related to '
                           'service discovery. It will not be used.')
 
@@ -409,7 +404,7 @@ def get_config(cfg_path=None, options=None, can_query_registry=True):
             'dogstatsd_port': 8125,
             'dogstatsd_target': 'http://' + agentConfig['bind_host'] + ':17123',
         }
-        for key, value in dogstatsd_defaults.iteritems():
+        for key, value in dogstatsd_defaults.items():
             if config.has_option('Main', key):
                 agentConfig[key] = config.get('Main', key)
             else:
@@ -500,11 +495,11 @@ def get_config(cfg_path=None, options=None, can_query_registry=True):
         if config.has_option("Main", "gce_updated_hostname"):
             agentConfig["gce_updated_hostname"] = _is_affirmative(config.get("Main", "gce_updated_hostname"))
 
-    except ConfigParser.NoSectionError as e:
+    except ConfigParser.NoSectionError:
         sys.stderr.write('Config file not found or incorrectly formatted.\n')
         sys.exit(2)
 
-    except ConfigParser.ParsingError as e:
+    except ConfigParser.ParsingError:
         sys.stderr.write('Config file not found or incorrectly formatted.\n')
         sys.exit(2)
 
@@ -618,11 +613,8 @@ def get_proxy(agentConfig):
 
 def main():
     res = {}
-    for k, v in get_config().iteritems():
-        if v == None:
-            res[k] = "None"
-        else:
-            res[k] = str(v)
+    for k, v in get_config().items():
+        res[k] = str(v)
     return json.dumps(res, sort_keys=True)
 
 

diff --git a/rtloader/test/python/datadog_checks/checks/__init__.py b/rtloader/test/python/datadog_checks/checks/__init__.py
@@ -1,3 +1,2 @@
 # replicate the backward compat redirection we have in integrations-core
-from ..base.checks import *
-
+from ..base.checks import *  # noqa: F401, F403
diff --git a/setup.cfg b/setup.cfg
@@ -0,0 +1,9 @@
+[flake8]
+select = B,C,E,F,G,W,B001,B003,B006,B007,B301,B305,B306,B902
+# Ignore:
+# - black-incompatible options: E203
+# - bugbear overlap: E722
+# - style options: W2,W3,W50,E111,E114,E117,E12,E2,E3,E5,E74
+ignore = E203,W2,W3,W50,E111,E114,E117,E12,E2,E3,E5,E74,E722
+exclude = .git,.github,.circleci,chocolatey,docs,google-marketplace,omnibus,pkg-config,releasenotes,vendor
+max-line-length = 120
diff --git a/tasks/__init__.py b/tasks/__init__.py
@@ -27,7 +27,19 @@
 
 
 from .go import fmt, lint, vet, cyclo, golangci_lint, deps, lint_licenses, generate_licenses, reset, generate
-from .test import test, integration_tests, lint_teamassignment, lint_releasenote, lint_milestone, lint_filenames, e2e_tests, make_kitchen_gitlab_yml, check_gitlab_broken_dependencies, install_shellcheck
+from .test import (
+    test,
+    integration_tests,
+    lint_teamassignment,
+    lint_releasenote,
+    lint_milestone,
+    lint_filenames,
+    lint_python,
+    e2e_tests,
+    make_kitchen_gitlab_yml,
+    check_gitlab_broken_dependencies,
+    install_shellcheck, 
+)
 from .build_tags import audit_tag_impact
 
 # the root namespace
@@ -49,6 +61,7 @@
 ns.add_task(lint_releasenote)
 ns.add_task(lint_milestone)
 ns.add_task(lint_filenames)
+ns.add_task(lint_python)
 ns.add_task(audit_tag_impact)
 ns.add_task(e2e_tests)
 ns.add_task(make_kitchen_gitlab_yml)

diff --git a/tasks/agent.py b/tasks/agent.py
@@ -450,7 +450,7 @@ def omnibus_build(ctx, iot=False, agent_binaries=False, log_level="info", base_d
             omnibus_elapsed = omnibus_done - omnibus_start
 
 
-        except Exception as e:
+        except Exception:
             if pfxfile:
                 os.remove(pfxfile)
             raise

diff --git a/tasks/android.py b/tasks/android.py
@@ -2,26 +2,21 @@
 Android namespaced tasks
 """
 from __future__ import print_function
-import glob
 import yaml
 import os
 import shutil
 import sys
-from distutils.dir_util import copy_tree
 
-import invoke
 from invoke import task
-from invoke.exceptions import Exit
 
-from .utils import bin_name, get_build_flags, load_release_versions, get_version
+from .utils import bin_name, get_build_flags, get_version
 from .utils import REPO_PATH
 from .build_tags import get_default_build_tags
-from .go import deps, generate
+from .go import generate
 
 # constants
 BIN_PATH = os.path.join(".", "bin", "agent")
 AGENT_TAG = "datadog/agent:master"
-from .agent import DEFAULT_BUILD_TAGS
 
 ANDROID_CORECHECKS = [
     "cpu",
@@ -133,11 +128,7 @@ def clean(ctx):
 @task
 def assetconfigs(ctx):
     # move the core check config
-    try:
-        shutil.rmtree(CORECHECK_CONFS_DIR)
-    except:
-        ## it's ok if the dir is not there
-        pass
+    shutil.rmtree(CORECHECK_CONFS_DIR, ignore_errors=True)
 
     files = {}
     files_list = []

diff --git a/tasks/bench.py b/tasks/bench.py
@@ -5,7 +5,6 @@
 import os
 import sys
 
-import invoke
 from invoke import task
 
 from .build_tags import get_default_build_tags