Merge lp:~stefan-schwarzburg/qreator/qreator_vendor_plugins into lp:qreator

[Schwarzburg]

The implementation is a simplified version of my comment on https://trello.com/card/add-vendor-infrastructure/4fc6a3556634e8557016945d/27 :

Each plugin is responsible for checking for vendor compatibility (since we have only one at the moment, this seemed the simplest way and is also in line with the plugin idea). If the vendor compatibilty is not met, the plugin raises an VendorError, which is caught by the importer infrastrucuter.

This has at the moment nothing to do with the colors of the icons.

Also: I was not able to test this on non-ubuntu distros, but they should normally not be named "Ubuntu", and therefore the test wether this is "Ubuntu" should fail...

[David Planella (dpm)]

Nice work as usual, I like the approach.

Comments inline, just minor things:

21 + print "%s disabled due to missing vendor infrastructure." % (name,)

I wonder if we might want to use the logging system and just flag this as a warning message.

43 +

Additional whitespace here, just a nitpick, but it might be worth not introducing it.

67 + pass

Rather than passing here, do we want to perhaps print the warning in the exception as a central place, and let the plugins override the message?

review: Needs Information

[lp:~stefan-schwarzburg/qreator/qreator_vendor_plugins]

170. By Schwarzburg on 2013-05-20

added the VendorError body which can now hold an error-value; replaced a print statement by a proper logging call.

[Schwarzburg (stefan-schwarzburg)]

I think changes should adress your comments.

- The logging is implemented this way because the order of plugin-imports and logging setup matters
- The ValueError now has a body with a message. The printing of that message is still left to the structure that catches the error. I think this is more the way it is supposed in python.
- I have removed the whitespace, but added three other :-)
  38: There was one blank line to separate code, so I have added another one at the end of the added lines.
  52-53: After an import line, there should be two blank lines...

review: Needs Resubmitting

[Schwarzburg (stefan-schwarzburg)]

review: Needs Information

[David Planella (dpm)]

Merged, thanks!

I made a few small changes before the merge: namely, protecting the USC database module import with a try/exception block and raising a VendorError, as otherwise, the import was executed and it would have raised an ImportError exception if not available in a given platform other than Ubuntu.

review: Approve

=== modified file 'qreator/__init__.py'
--- qreator/__init__.py	2012-11-22 12:37:55 +0000
+++ qreator/__init__.py	2013-05-20 06:15:31 +0000
@@ -19,8 +19,9 @@
 from qreator_lib.i18n import _
 from gi.repository import Gtk, Gdk, GLib, GObject  # pylint: disable=E0611
 from qreator import QreatorWindow
-from qreator_lib import set_up_logging, get_version
+from qreator_lib import set_up_logging, get_version, VendorError
 from qreator.qrcodes.QRCodeType import QRCodeType
+import logging
 
 # use dynamic importing to get all qrcode dataformats without
 # knowing their names.
@@ -29,10 +30,13 @@
 import pkgutil
 import qreator.qrcodes
 
+warnings = []
 for qr in list(pkgutil.iter_modules(qreator.qrcodes.__path__)):
     name = "qreator.qrcodes." + qr[1]
-    __import__(name, fromlist=[])
-
+    try:
+        __import__(name, fromlist=[])
+    except VendorError as e:
+        warnings.append("%s: %s." % (name, e))
 
 def UTF8_(message):
     return _(message).decode('UTF-8')
@@ -65,6 +69,10 @@
     qr_types = [d(window.update_qr_code) for d in QRCodeType.dataformats]
     options = parse_options(qr_types)
 
+    logger = logging.getLogger('qreator')
+    for warning_message in warnings:
+        logger.warn(warning_message)
+    
     window.qr_types = qr_types
     window.show()
     if getattr(options, 'view', None) is not None:

=== modified file 'qreator/qrcodes/QRCodeSoftwareCenterApp.py'
--- qreator/qrcodes/QRCodeSoftwareCenterApp.py	2012-11-23 08:34:02 +0000
+++ qreator/qrcodes/QRCodeSoftwareCenterApp.py	2013-05-20 06:15:31 +0000
@@ -14,6 +14,14 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 ### END LICENSE
 
+from qreator_lib import VendorError
+import platform
+
+
+# this is a vendor specific plugin
+if not platform.linux_distribution()[0] == "Ubuntu":
+    raise VendorError
+
 from qreator_lib.i18n import _
 from QRCodeType import QRCodeType
 from QRCodeSoftwareCenterAppGtk import QRCodeSoftwareCenterAppGtk

=== modified file 'qreator_lib/__init__.py'
--- qreator_lib/__init__.py	2012-11-22 08:43:25 +0000
+++ qreator_lib/__init__.py	2013-05-20 06:15:31 +0000
@@ -22,4 +22,4 @@
 from . helpers import set_up_logging
 from . Window import Window
 from . qreatorconfig import get_version
-
+from . helpers import VendorError

=== modified file 'qreator_lib/helpers.py'
--- qreator_lib/helpers.py	2013-05-08 18:21:38 +0000
+++ qreator_lib/helpers.py	2013-05-20 06:15:31 +0000
@@ -26,6 +26,16 @@
 
 from locale import gettext as _
 
+class VendorError(Exception):
+    """Raised by plugins if the required vendor infractucture is missing"""
+    def __init__(self, value=None):
+        if value is None:
+            value = "Disabled due to missing vendor infrastructure."
+        self.value = value
+    def __str__(self):
+        return repr(self.value)
+
+
 def get_builder(builder_file_name):
     """Return a fully-instantiated Gtk.Builder instance from specified ui
     file