Merge lp:~jtv/maas/bug-1359517 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Don't use maaslog.exception; use maaslog.error instead.

Apparently maaslog.exception is a complete no-no. So I added a lint check that forbids it; testing against every possible call is a losing game.

Most lint checks have the logic for finding source files right in the Makefile, but that makes things a bit verbose. Since I needed to customise the set of source files, I hid that part away in my checker script.

Jeroen

[Julian Edwards (julian-edwards)]

Nice.

review: Approve

[Jeroen T. Vermeulen (jtv)]

> Nice.

Apart from the fact that it's mostly shell script, of course.

=== modified file 'Makefile'
--- Makefile	2014-08-18 13:44:26 +0000
+++ Makefile	2014-08-21 05:24:11 +0000
@@ -140,6 +140,7 @@
 lint-py: bin/flake8
 	@find $(sources) -name '*.py' ! -path '*/migrations/*' \
 	    -print0 | xargs -r0 bin/flake8 --ignore=E123 --config=/dev/null
+	@./utilities/check-maaslog-exception
 
 lint-doc:
 	@./utilities/doc-lint

=== modified file 'src/provisioningserver/dhcp/detect.py'
--- src/provisioningserver/dhcp/detect.py	2014-08-13 21:49:35 +0000
+++ src/provisioningserver/dhcp/detect.py	2014-08-21 05:24:11 +0000
@@ -359,7 +359,7 @@
         try:
             servers = probe_interface(interface, ip)
         except socket.error:
-            maaslog.exception(
+            maaslog.error(
                 "Failed to probe sockets; did you configure authbind as per "
                 "HACKING.txt?")
             return

=== added file 'utilities/check-maaslog-exception'
--- utilities/check-maaslog-exception	1970-01-01 00:00:00 +0000
+++ utilities/check-maaslog-exception	2014-08-21 05:24:11 +0000
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Check for use of "maaslog.exception" in the source tree.  Code should use
+# "maaslog.error" instead.
+#
+# Usage: check-maaslog-exception [branch]
+#
+# Where branch is an optional branch; defaults to the current directory.
+
+# Exit immediately if a command exits with a non-zero status.
+set -o errexit
+# Treat unset variables as an error when substituting.
+set -o nounset
+
+
+# Look for use of maaslog.exception in the source tree.  Not tests, since
+# they may also look for this mistake.
+find_incorrect_usage() {
+    # Find all Python files in src, except tests,
+    # which have "maaslog.exception(" on a line that isn't a comment.
+    find $1/src -name \*.py ! -path '*/tests/*' -print0 |
+        xargs -r0 grep -n '^[^#]*\<maaslog\.exception('
+}
+
+
+main() {
+    local branch="${1:-.}"
+    local bad_calls="$(find_incorrect_usage "$branch")"
+    if test -n "${bad_calls}"
+    then
+        cat >&2 <<EOF
+Code appears to call maaslog.exception:
+
+${bad_calls}
+
+Use maaslog.error instead.
+EOF
+        exit 1
+    fi
+}
+
+
+main