Apply pylint and isort, and add a "quality" build

.isort.cfg

@@ -0,0 +1,16 @@
+[isort]
+multi_line_output=3
+include_trailing_comma=True
+force_grid_wrap=0
+combine_as_imports=True
+line_length=88
+lines_after_imports = 2
+force_single_line = true
+force_sort_within_sections = true
+sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,TESTS,LOCALFOLDER
+known_tests =
+    tests
+skip =
+    docs/conf.py
+skip_glob =
+    */migrations/*

.pylintrc

@@ -0,0 +1,90 @@
+[MASTER]
+
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code
+extension-pkg-whitelist=lxml,posix_ipc,spidev,netifaces
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=migrations,south_migrations
+
+# Add files or directories matching the regex patterns to the blacklist. The
+# regex matches against base names, not paths.
+ignore-patterns=.*_pb2.py
+
+
+[MESSAGES CONTROL]
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once).You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use"--disable=all --enable=classes
+# --disable=W"
+disable=
+    arguments-differ,
+    attribute-defined-outside-init,
+    bad-continuation,
+    cyclic-import,
+    duplicate-code,
+    fixme,
+    file-ignored,
+    invalid-name,
+    locally-enabled,
+    locally-disabled,
+    missing-docstring,
+    no-init,
+    no-member,
+    no-self-use,
+    old-style-class,
+    protected-access,
+    redefined-variable-type,
+    superfluous-parens,
+    too-few-public-methods,
+    too-many-ancestors,
+    too-many-arguments,
+    too-many-branches,
+    too-many-instance-attributes,
+    too-many-lines,
+    too-many-locals,
+    too-many-public-methods,
+    too-many-statements,
+    unused-argument,
+    useless-return,
+    wrong-import-order  # we have a custom isort config which pylint can't grok
+
+
+[REPORTS]
+
+# Template used to display messages. This is a python new-style format string
+# used to format the message information. See doc for all details
+msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"
+
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+max-line-length=120
+
+
+[VARIABLES]
+
+# A regular expression matching the beginning of the name of dummy variables
+# (i.e. not used).
+# Don't use pylint 1.6.1 default regexp, it doesn't accept underscores in the middle
+dummy-variables-rgx=_|dummy
+
+[DESIGN]
+
+# Maximum number of arguments for function / method
+max-args=8
+
+# Maximum number of parents for a class (see R0901).
+max-parents=10
+
+# Maximum number of attributes for a class (see R0902).
+max-attributes=10

.travis.yml

@@ -19,6 +19,7 @@ matrix:
 
   include:
     - { python: "3.6", env: TOXENV=docs }
+    - { python: "3.6", env: TOXENV=quality }
 
   exclude:
     - { python: "3.4", env: DJANGO=2.1 }

Makefile

@@ -10,5 +10,9 @@ clean:
 docs:
 	sphinx-build -W -n -b html docs ./build/sphinx/html
 
+quality:
+	python setup.py check --strict --metadata --restructuredtext
+	pylint --reports=no setup.py cid
+
 test:
 	py.test tests

cid/backends/mysql/base.py

@@ -1,9 +1,11 @@
-from django.db.backends.mysql.base import DatabaseWrapper as BaseMySQLWrapper
+from django.db.backends.mysql.base import DatabaseWrapper as BaseDatabaseWrapper
 
 from ...cursor import CidCursorWrapper
 
 
-class DatabaseWrapper(BaseMySQLWrapper):
+class DatabaseWrapper(BaseDatabaseWrapper):
+    # Don't warn about abstract `_start_transaction_under_autocommit`
+    # pylint: disable=abstract-method
 
     def create_cursor(self, name=None):
         base_cursor = super().create_cursor(name)

cid/backends/oracle/base.py

@@ -1,11 +1,11 @@
-from django.db.backends.oracle.base import (
-    DatabaseWrapper as BaseOracleWrapper
-)
+from django.db.backends.oracle.base import DatabaseWrapper as BaseDatabaseWrapper
 
 from ...cursor import CidCursorWrapper
 
 
-class DatabaseWrapper(BaseOracleWrapper):
+class DatabaseWrapper(BaseDatabaseWrapper):
+    # Don't warn about abstract `_start_transaction_under_autocommit`
+    # pylint: disable=abstract-method
 
     def create_cursor(self, name=None):
         base_cursor = super().create_cursor(name)

cid/backends/postgis/base.py

@@ -1,11 +1,12 @@
-from django.contrib.gis.db.backends.postgis.base import (
-    DatabaseWrapper as BasePostgisWrapper
-)
+from django.contrib.gis.db.backends.postgis.base import \
+    DatabaseWrapper as BaseDatabaseWrapper
 
 from ...cursor import CidCursorWrapper
 
 
-class DatabaseWrapper(BasePostgisWrapper):
+class DatabaseWrapper(BaseDatabaseWrapper):
+    # Don't warn about abstract `_start_transaction_under_autocommit`
+    # pylint: disable=abstract-method
 
     def create_cursor(self, name=None):
         base_cursor = super().create_cursor(name)

cid/backends/postgresql/base.py

@@ -1,11 +1,12 @@
-from django.db.backends.postgresql_psycopg2.base import (
-    DatabaseWrapper as BasePostgresqlWrapper
-)
+from django.db.backends.postgresql_psycopg2.base import \
+    DatabaseWrapper as BaseDatabaseWrapper
 
 from ...cursor import CidCursorWrapper
 
 
-class DatabaseWrapper(BasePostgresqlWrapper):
+class DatabaseWrapper(BaseDatabaseWrapper):
+    # Don't warn about abstract `_start_transaction_under_autocommit`
+    # pylint: disable=abstract-method
 
     def create_cursor(self, name=None):
         base_cursor = super().create_cursor(name)

cid/backends/sqlite3/base.py

@@ -1,11 +1,9 @@
-from django.db.backends.sqlite3.base import (
-    DatabaseWrapper as BaseSqliteWrapper
-)
+from django.db.backends.sqlite3.base import DatabaseWrapper as BaseDatabaseWrapper
 
 from ...cursor import CidCursorWrapper
 
 
-class DatabaseWrapper(BaseSqliteWrapper):
+class DatabaseWrapper(BaseDatabaseWrapper):
 
     def create_cursor(self, name=None):
         base_cursor = super().create_cursor(name)

cid/cursor.py

@@ -1,4 +1,5 @@
 from django.conf import settings
+
 from .locals import get_cid
 
 
@@ -15,27 +16,29 @@ def __init__(self, cursor):
     def __getattr__(self, attr):
         if attr in self.__dict__:
             return self.__dict__[attr]
-        else:
-            return getattr(self.cursor, attr)
+        return getattr(self.cursor, attr)
 
     def __iter__(self):
         return iter(self.cursor)
 
     def __enter__(self):
         return self
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exc_type, exc_value, traceback):
         self.close()
 
     def add_comment(self, sql):
         cid_sql_template = getattr(
             settings, 'CID_SQL_COMMENT_TEMPLATE', DEFAULT_CID_SQL_COMMENT_TEMPLATE
         )
         cid = get_cid()
-        if cid:
-            cid = cid.replace('/*', '\/\*').replace('*/', '\*\/')
-            return "/* {} */\n{}".format(cid_sql_template.format(cid=cid), sql)
-        return sql
+        if not cid:
+            return sql
+        # FIXME (dbaty): we could use "--" prefixed comments so that
+        # we would not have to bother with escaping the cid (assuming
+        # it does not contain newline characters).
+        cid = cid.replace('/*', r'\/\*').replace('*/', r'\*\/')
+        return "/* {} */\n{}".format(cid_sql_template.format(cid=cid), sql)
 
     # The following methods cannot be implemented in __getattr__, because the
     # code must run when the method is invoked, not just when it is accessed.

cid/locals.py

@@ -3,6 +3,7 @@
 
 from django.conf import settings
 
+
 _thread_locals = local()
 
 

cid/middleware.py

@@ -1,6 +1,7 @@
 from django.conf import settings
 
-from .locals import set_cid, get_cid
+from cid.locals import get_cid
+from cid.locals import set_cid
 
 
 class CidMiddleware:

requirements/quality.txt

@@ -0,0 +1,4 @@
+# `docutils` is needed to check for `setup.py check --restructuredtext`
+docutils
+isort
+pylint

setup.py

@@ -10,6 +10,7 @@
     description="""Correlation IDs in Django for debugging requests""",
     long_description=readme + '\n\n' + history,
     author='Snowball One',
+    author_email='please-direct-your-queries-to-the-maintainer',
     maintainer="Polyconseil",
     maintainer_email="opensource+django-cid@polyconseil.fr",
     url='https://github.com/Polyconseil/django-cid',

tests/test_cursor.py

@@ -3,6 +3,7 @@
 from django.test import TestCase
 from django.test.utils import override_settings
 
+import cid.locals
 from cid.cursor import CidCursorWrapper
 
 
@@ -53,3 +54,9 @@ def test_executemany_calls_add_comment(self, add_comment):
         sql = "SELECT 1;"
         self.cursor_wrapper.executemany(sql, [])
         add_comment.assert_called_with(sql)
+
+    @mock.patch('cid.cursor.get_cid')
+    def test_escape_cid(self, get_cid):
+        get_cid.return_value = '/* a correlation id with funny characters */'
+        expected = '/* cid: \\/\\* a correlation id with funny characters \\*\\/ */\nSELECT 1;'
+        self.assertEqual(self.cursor_wrapper.add_comment('SELECT 1;'), expected)

tox.ini

@@ -3,7 +3,8 @@ envlist =
     py{34,35,36}-django111,
     py{34,35,36}-django20,
     py{35,36}-django21,
-    docs
+    docs,
+    quality
 
 [travis:env]
 DJANGO =
@@ -25,3 +26,9 @@ basepython = python3.6
 commands = /usr/bin/make docs
 deps =
     -rrequirements/docs.txt
+
+[testenv:quality]
+basepython = python3.6
+commands = /usr/bin/make quality
+deps =
+    -rrequirements/quality.txt