Code review comment for lp:~leonardr/lazr.restfulclient/system-wide-consumer

Hi Leonard,

This is a nice branch. I just have one concern listed below.

-Edwin

>=== modified file 'src/lazr/restfulclient/authorize/oauth.py'
>--- src/lazr/restfulclient/authorize/oauth.py 2010-10-28 15:49:09 +0000
>+++ src/lazr/restfulclient/authorize/oauth.py 2010-10-28 20:57:14 +0000
>@@ -73,6 +76,49 @@
> self.context = context
>
>
>+class SystemWideConsumer(Consumer):
>+ """A consumer associated with the logged-in user rather than an app.
>+
>+ This can be used to share a single OAuth token among multiple
>+ desktop applications. The OAuth consumer key will be derived from
>+ system information (platform and hostname).
>+ """
>+ KEY_FORMAT = "System-wide: %s (%s)"
>+
>+ def __init__(self, application_name, secret=''):
>+ """Constructor.
>+
>+ :param application_name: An application name. This will be
>+ used in the User-Agent header.
>+ :param secret: The OAuth consumer secret. Don't use this. It's
>+ a misfeature, and lazr.restful doesn't expect it.
>+ """
>+ super(SystemWideConsumer, self).__init__(
>+ self.consumer_key, secret, application_name)
>+
>+ @property
>+ def consumer_key(self):
>+ """The system-wide OAuth consumer key for this computer.
>+
>+ This key identifies the platform and the computer's
>+ hostname. It does not identify the active user.
>+ """
>+ try:
>+ distname, version, release_id = platform.linux_distribution()
>+ except Exception, e:
>+ # This can happen in pre-2.6 versions of Python.
>+ try:
>+ distname, version, release_id = platform.dist()

There appears to be an annoying bug in some versions of python where platform.dist() will randomly get its data from either /etc/debian_version or /etc/lsb-release. Since lsb-release has the more accurate info, it might be worthwhile to use "/usr/bin/lsb_release -si" when that executable exists.

http://bugs.python.org/issue9514

>+ except Exception, e:
>+ # This should never happen--non-Linux platforms return
>+ # empty strings from linux_distribution() or
>+ # dist()--but just in case.
>+ distname = ''
>+ if distname == '':
>+ distname = platform.system() # (eg. "Windows")
>+ return self.KEY_FORMAT % (distname, socket.gethostname())
>+
>+