Merge lp:~pfalcon/linaro-ci-dashboard/logging into lp:linaro-ci-dashboard

[Paul Sokolovsky]

Add (proper) logging support for linaro-ci-dashboard. Example:

2012-08-17 04:45:12,361 INFO [jenkinsserver.JenkinsServer] Creating/updating Jenkins job for loop: IntegrationLoop(id=1, branch=lp:linaro-android-build-tools, ...)

Note that helpers.py is intended as catch-all for all other helper things we may need, that's why it's named though and put at the top level.

[Stevan Radaković (stevanr)]

Hi Paul, thanks for implementing the logging support!!

One small change is needed though...
Since we'er following the OO concept to the core (we even have class-based views in apps), let's try to respect it now as well.
My idea is to create a 'lib' directory under dashboard and add the i.e. Logger class over there with static 'getClassLogger' method.. Then do the same thing you're doing right now, meaning everything else looks good.

review: Needs Fixing

[Paul Sokolovsky (pfalcon)]

Well, artificially wrapping standalone functions in once-off classes in not really OO, nor Pythonic way of doing things - unlike Java, with Python, there's no need (nor there's a pattern of) wrapping everything in classes. I'm going to make as you suggest in this case, but this whole matter needs more discussion.

[Stevan Radaković (stevanr)]

On 08/17/2012 02:13 PM, Paul Sokolovsky wrote:
> Well, artificially wrapping standalone functions in once-off classes in not really OO, nor Pythonic way of doing things - unlike Java, with Python, there's no need (nor there's a pattern of) wrapping everything in classes. I'm going to make as you suggest in this case, but this whole matter needs more discussion.
>

Yes I was thinking of a Logger class which would actually contain
self.log as an attribute and then all the log-able classes could inherit
this class and use the self.log outright... But as I played with this I
realized that most of our relevant classes already inherit other django
classes like Model etc. and I had problems with multiple inheritance in
my quick examples (i.e. calling __init__ for both base classes), so I
just suggested the next best thing...

[lp:~pfalcon/linaro-ci-dashboard/logging]

25. By Paul Sokolovsky on 2012-08-17

Move helpers.getClassLogger() to lib.logger.Logger.getClassLogger().

[Paul Sokolovsky (pfalcon)]

> Yes I was thinking of a Logger class which would actually contain
self.log as an attribute and then all the log-able classes could inherit
this class and use the self.log outright...

Well, this would make much more sense, and I already regretted that I didn't strongly suggest to just use module-level logger in my RFC. Because for constructor-less classes, we'd now need to add constructor, and due to Python's verbose super() syntax, that really sucks.

But yeah, you can easily mix-in a behavior (i.e. standalone methods), but not so easy a state, which would require a constructor to initialize.

Anyway, the refactor is ready.

[Stevan Radaković (stevanr)]

Looks good.
Approve +1.

review: Approve

=== modified file 'dashboard/frontend/integration_loop/models/integration_loop.py'
--- dashboard/frontend/integration_loop/models/integration_loop.py	2012-08-16 15:12:10 +0000
+++ dashboard/frontend/integration_loop/models/integration_loop.py	2012-08-17 13:44:39 +0000
@@ -45,3 +45,6 @@
         # loop config on the remote server.
         self.server.create_or_update_integration_loop(self)
         super(IntegrationLoop, self).schedule_build()
+
+    def __repr__(self):
+        return "IntegrationLoop(id=%d, branch=%s, ...)" % (self.id, self.branch)

=== modified file 'dashboard/jenkinsserver/models/jenkins_server.py'
--- dashboard/jenkinsserver/models/jenkins_server.py	2012-07-26 13:25:06 +0000
+++ dashboard/jenkinsserver/models/jenkins_server.py	2012-08-17 13:44:39 +0000
@@ -22,6 +22,7 @@
 import jenkins
 import json
 import urllib2
+from lib.logger import Logger
 
 class JenkinsServer(models.Model):
     class Meta:
@@ -33,6 +34,7 @@
 
     def __init__(self, *args, **kwargs):
         super(JenkinsServer, self).__init__(*args, **kwargs)
+        self.log = Logger.getClassLogger(self)
         self.jenkins = jenkins.Jenkins(self.url,
                                        self.username.encode('utf-8'),
                                        self.password.encode('utf-8'))
@@ -41,6 +43,7 @@
         """ Create integration loop if it doesn't exist on Jenkins.
         Otherwise, update the existing one with new XML."""
 
+        self.log.info("Creating/updating Jenkins job for loop: %r", loop)
         jxml = self.create_integration_job_xml(loop)
         if not self.jenkins.job_exists(loop.name):
             self.create_job(loop.name, jxml)

=== added directory 'dashboard/lib'
=== added file 'dashboard/lib/__init__.py'
=== added file 'dashboard/lib/logger.py'
--- dashboard/lib/logger.py	1970-01-01 00:00:00 +0000
+++ dashboard/lib/logger.py	2012-08-17 13:44:39 +0000
@@ -0,0 +1,30 @@
+# Copyright (C) 2012 Linaro
+#
+# This file is part of linaro-ci-dashboard.
+#
+# linaro-ci-dashboard is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# linaro-ci-dashboard is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with linaro-ci-dashboard.  If not, see <http://www.gnu.org/licenses/>.
+# Django settings for dashboard project.
+
+import logging
+
+
+class Logger:
+    "Logging-related helper class."
+
+    @staticmethod
+    def getClassLogger(obj):
+        "Given a class instance, return logger to use for that class."
+        toplevel_package = obj.__class__.__module__.split(".", 1)[0]
+        logger_name = toplevel_package + '.' + obj.__class__.__name__
+        return logging.getLogger(logger_name)

=== modified file 'dashboard/settings.py'
--- dashboard/settings.py	2012-08-17 07:32:04 +0000
+++ dashboard/settings.py	2012-08-17 13:44:39 +0000
@@ -170,7 +170,17 @@
 LOGGING = {
     'version': 1,
     'disable_existing_loggers': False,
+    'formatters': {
+        'default': {
+            'format': '%(asctime)s %(levelname)s [%(name)s] %(message)s'
+        },
+    },
     'handlers': {
+        'console': {
+            'class': 'logging.StreamHandler',
+            'stream': 'ext://sys.stdout',
+            'formatter': 'default',
+        },
         'mail_admins': {
             'level': 'ERROR',
             'class': 'django.utils.log.AdminEmailHandler'
@@ -182,6 +192,10 @@
             'level': 'ERROR',
             'propagate': True,
         },
+    },
+    'root': {
+        'level': 'INFO',
+        'handlers': ['console'],
     }
 }