From 09fb740298c834a07ad6064b7603a377cc684ed4 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Mon, 22 Sep 2014 11:35:29 +0200 Subject: [PATCH] Use django.utils.module_loading.import_by_path to import synchronizers #178 --- .../management/commands/synchronize.py | 62 +++++++++---------- .../interoperability/models/layer_external.py | 8 +-- nodeshot/interoperability/settings.py | 24 +++---- nodeshot/interoperability/tasks.py | 21 +++---- nodeshot/interoperability/tests.py | 40 ++++++------ 5 files changed, 72 insertions(+), 83 deletions(-) diff --git a/nodeshot/interoperability/management/commands/synchronize.py b/nodeshot/interoperability/management/commands/synchronize.py index f27ab860..27c26a6b 100755 --- a/nodeshot/interoperability/management/commands/synchronize.py +++ b/nodeshot/interoperability/management/commands/synchronize.py @@ -1,17 +1,17 @@ from django.core.management.base import BaseCommand, CommandError from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from django.db.models import Q +from django.utils.module_loading import import_by_path from nodeshot.core.layers.models import Layer -from importlib import import_module from optparse import make_option class Command(BaseCommand): args = '' help = 'Synchronize external layers with the local database' - + option_list = BaseCommand.option_list + ( make_option( '--exclude', @@ -29,15 +29,15 @@ def retrieve_layers(self, *args, **options): """ Retrieve specified layers or all external layers if no layer specified. """ - + # init empty Q object queryset = Q() - + # if no layer specified if len(args) < 1: # cache queryset all_layers = Layer.objects.published().external() - + # check if there is any layer to exclude if options['exclude']: # convert comma separated string in python list, ignore spaces @@ -48,88 +48,84 @@ def retrieve_layers(self, *args, **options): # nothing to exclude, retrieve all layers self.verbose('no layer specified, will retrieve all layers!') return all_layers - + # otherwise loop over args and retrieve each specified layer for layer_slug in args: queryset = queryset | Q(slug=layer_slug) - + # verify existence try: # retrieve layer layer = Layer.objects.get(slug=layer_slug) - + # raise exception if layer is not external if not layer.is_external: raise CommandError('Layer "%s" is not an external layer\n\r' % layer_slug) - + # raise exception if layer is not published if not layer.is_published: raise CommandError('Layer "%s" is not published. Why are you trying to work on an unpublished layer?\n\r' % layer_slug) - + # raise exception if one of the layer looked for doesn't exist except Layer.DoesNotExist: raise CommandError('Layer "%s" does not exist\n\r' % layer_slug) - + # return published external layers return Layer.objects.published().external().select_related().filter(queryset) - + def verbose(self, message): if self.verbosity == 2: - self.stdout.write('%s\n\r' % message) + self.stdout.write('%s\n\r' % message) def handle(self, *args, **options): """ execute synchronize command """ # store verbosity level in instance attribute for later use self.verbosity = int(options.get('verbosity')) - + # blank line self.stdout.write('\r\n') - + # retrieve layers layers = self.retrieve_layers(*args, **options) - + if len(layers) < 1: self.stdout.write('no layers to process\n\r') return else: self.verbose('going to process %d layers...' % len(layers)) - + # loop over for layer in layers: # retrieve interop class if available try: - interop = layer.external.interoperability + synchronizer_path = layer.external.interoperability except (ObjectDoesNotExist, AttributeError): self.stdout.write('External Layer %s does not have an interoperability class specified\n\r' % layer.name) continue - - # if no interop jump to next layer - if interop == 'None': + + # if no synchronizer_path jump to next layer + if synchronizer_path == 'None': self.stdout.write('External Layer %s does not have an interoperability class specified\n\r' % layer.name) continue - + if layer.external.config is None: self.stdout.write('Layer %s does not have a config yet\n\r' % layer.name) continue - - # else go ahead and import module - interop_module = import_module(interop) - # retrieve class name (split and get last piece) - class_name = interop.split('.')[-1] + # retrieve class - interop_class = getattr(interop_module, class_name) - self.stdout.write('imported module %s\r\n' % interop_module.__file__) - + Synchronizer = import_by_path(synchronizer_path) + self.stdout.write('imported module %s\r\n' % Synchronizer.__name__) + # try running try: - instance = interop_class(layer, verbosity=self.verbosity) + instance = Synchronizer(layer, verbosity=self.verbosity) self.stdout.write('Processing layer "%s"\r\n' % layer.slug) messages = instance.process() except ImproperlyConfigured, e: self.stdout.write('Validation error: %s\r\n' % e) continue - + for message in messages: self.stdout.write('%s\n\r' % message) - + self.stdout.write('\r\n') diff --git a/nodeshot/interoperability/models/layer_external.py b/nodeshot/interoperability/models/layer_external.py index 6586818a..681db1bb 100755 --- a/nodeshot/interoperability/models/layer_external.py +++ b/nodeshot/interoperability/models/layer_external.py @@ -1,9 +1,9 @@ -from importlib import import_module import simplejson as json from django.db import models from django.utils.translation import ugettext_lazy as _ from django.core.exceptions import ValidationError, ImproperlyConfigured +from django.utils.module_loading import import_by_path from ..settings import SYNCHRONIZERS @@ -119,11 +119,7 @@ def synchronizer_class(self): self._synchronizer_class = None if not self._synchronizer_class: - synchronizer_module = import_module(self.interoperability) - # retrieve class name (split and get last piece) - class_name = self.interoperability.split('.')[-1] - # retrieve class - self._synchronizer_class = getattr(synchronizer_module, class_name) + self._synchronizer_class = import_by_path(self.interoperability) return self._synchronizer_class diff --git a/nodeshot/interoperability/settings.py b/nodeshot/interoperability/settings.py index c9308107..314dc38e 100755 --- a/nodeshot/interoperability/settings.py +++ b/nodeshot/interoperability/settings.py @@ -2,18 +2,18 @@ DEFAULT_SYNCHRONIZERS = [ - ('nodeshot.interoperability.synchronizers.Nodeshot', 'Nodeshot'), - ('nodeshot.interoperability.synchronizers.GeoJson', 'GeoJSON'), - ('nodeshot.interoperability.synchronizers.GeoRss', 'GeoRSS'), - ('nodeshot.interoperability.synchronizers.OpenWisp', 'OpenWisp'), - ('nodeshot.interoperability.synchronizers.OpenWispCitySdkTourism', 'OpenWispCitySdkTourism'), - ('nodeshot.interoperability.synchronizers.ProvinciaWifi', 'Provincia WiFi'), - ('nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkTourism', 'ProvinciaWifiCitySdkTourism'), - ('nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkMobility', 'Synchronize Provincia Wifi with CitySDK Mobility'), - ('nodeshot.interoperability.synchronizers.CitySdkMobility', 'CitySDK Mobility (event driven)'), - ('nodeshot.interoperability.synchronizers.GeoJsonCitySdkMobility', 'Import GeoJSON into CitySDK Mobility API'), - ('nodeshot.interoperability.synchronizers.GeoJsonCitySdkTourism', 'Import GeoJSON into CitySDK Tourism API'), - ('nodeshot.interoperability.synchronizers.OpenLabor', 'OpenLabor'), + ('nodeshot.interoperability.synchronizers.Nodeshot.Nodeshot', 'Nodeshot'), + ('nodeshot.interoperability.synchronizers.GeoJson.GeoJson', 'GeoJSON'), + ('nodeshot.interoperability.synchronizers.GeoRss.GeoRss', 'GeoRSS'), + ('nodeshot.interoperability.synchronizers.OpenWisp.OpenWisp', 'OpenWisp'), + ('nodeshot.interoperability.synchronizers.OpenWispCitySdkTourism.OpenWispCitySdkTourism', 'OpenWispCitySdkTourism'), + ('nodeshot.interoperability.synchronizers.ProvinciaWifi.ProvinciaWifi', 'Provincia WiFi'), + ('nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkTourism.ProvinciaWifiCitySdkTourism', 'ProvinciaWifiCitySdkTourism'), + ('nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkMobility.ProvinciaWifiCitySdkMobility', 'Synchronize Provincia Wifi with CitySDK Mobility'), + ('nodeshot.interoperability.synchronizers.CitySdkMobility.CitySdkMobility', 'CitySDK Mobility (event driven)'), + ('nodeshot.interoperability.synchronizers.GeoJsonCitySdkMobility.GeoJsonCitySdkMobility', 'Import GeoJSON into CitySDK Mobility API'), + ('nodeshot.interoperability.synchronizers.GeoJsonCitySdkTourism.GeoJsonCitySdkTourism', 'Import GeoJSON into CitySDK Tourism API'), + ('nodeshot.interoperability.synchronizers.OpenLabor.OpenLabor', 'OpenLabor'), ] SYNCHRONIZERS = DEFAULT_SYNCHRONIZERS + getattr(settings, 'NODESHOT_SYNCHRONIZERS', []) diff --git a/nodeshot/interoperability/tasks.py b/nodeshot/interoperability/tasks.py index fa630c02..c6576b0a 100755 --- a/nodeshot/interoperability/tasks.py +++ b/nodeshot/interoperability/tasks.py @@ -1,5 +1,5 @@ from celery import task -from importlib import import_module +from django.utils.module_loading import import_by_path from django.core import management @@ -20,7 +20,7 @@ def push_changes_to_external_layers(node, external_layer, operation): Sync other applications through their APIs by performing updates, adds or deletes. This method is designed to be performed asynchronously, avoiding blocking the user when he changes data on the local DB. - + :param node: the node which should be updated on the external layer. :type node: Node model instance :param operation: the operation to perform (add, change, delete) @@ -30,19 +30,16 @@ def push_changes_to_external_layers(node, external_layer, operation): # subsequent imports go and look into sys.modules before reimporting the module again # so performance is not affected from nodeshot.core.nodes.models import Node - + # get node because for some reason the node instance object is not passed entirely, # pheraphs because objects are serialized by celery or transport/queuing mechanism if not isinstance(node, basestring): node = Node.objects.get(pk=node.pk) - - interop_module = import_module(external_layer.interoperability) - # retrieve class name (split and get last piece) - class_name = external_layer.interoperability.split('.')[-1] - # retrieve class - interop_class = getattr(interop_module, class_name) - instance = interop_class(external_layer.layer) - + + # import synchronizer + Synchronizer = import_by_path(external_layer.interoperability) + instance = Synchronizer(external_layer.layer) + # call method only if supported if hasattr(instance, operation): - getattr(instance, operation)(node) \ No newline at end of file + getattr(instance, operation)(node) diff --git a/nodeshot/interoperability/tests.py b/nodeshot/interoperability/tests.py index fc544ba8..181a7ce6 100755 --- a/nodeshot/interoperability/tests.py +++ b/nodeshot/interoperability/tests.py @@ -115,7 +115,7 @@ def test_layer_admin(self): self.assertEqual(response.status_code, 200) external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot' + external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot.Nodeshot' external.config = '{ "layer_url": "http://test.com/" }' external.save() @@ -135,7 +135,7 @@ def test_config_validation(self): layer.save() layer = Layer.objects.get(pk=layer.pk) external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWisp' + external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWisp.OpenWisp' external.config = '{ "WRONG_parameter_name": "foo" }' with self.assertRaises(ValidationError): @@ -152,7 +152,7 @@ def test_admin_synchronize_action(self): url = '%s/geojson1.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson.GeoJson' external.config = '{ "url": "%s", "map": {} }' % url external.full_clean() external.save() @@ -207,7 +207,7 @@ def test_openwisp(self): xml_url = '%s/openwisp-georss.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWisp' + external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWisp.OpenWisp' external.config = '{ "url": "%s" }' % xml_url external.save() @@ -281,7 +281,7 @@ def test_provinciawifi(self): xml_url = '%s/provincia-wifi.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifi' + external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifi.ProvinciaWifi' external.config = '{ "url": "%s" }' % xml_url external.save() @@ -360,7 +360,7 @@ def test_province_rome_traffic(self): measurements_url = '%s/citysdk-wp4-measurements.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinceRomeTraffic' + external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinceRomeTraffic.ProvinceRomeTraffic' external.config = json.dumps({ "streets_url": streets_url, "measurements_url": measurements_url, @@ -467,7 +467,7 @@ def test_geojson_sync(self): url = '%s/geojson1.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson.GeoJson' external.config = '{ "url": "%s", "map": {} }' % url external.full_clean() external.save() @@ -547,7 +547,7 @@ def test_preexisting_name(self): url = '%s/geojson1.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson.GeoJson' external.config = '{ "url": "%s", "map": {} }' % url external.full_clean() external.save() @@ -581,7 +581,7 @@ def test_key_mappings(self): url = '%s/geojson3.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJson.GeoJson' external.config = json.dumps({ "url": url, "map": { @@ -630,7 +630,7 @@ def test_georss_simple(self): url = '%s/georss-simple.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoRss' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoRss.GeoRss' external.config = '{ "url": "%s", "map": {} }' % url external.full_clean() external.save() @@ -688,7 +688,7 @@ def test_georss_w3c(self): url = '%s/georss-w3c.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoRss' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoRss.GeoRss' external.config = '{ "url": "%s", "map": {} }' % url external.full_clean() external.save() @@ -743,7 +743,7 @@ def test_openlabor_get_nodes(self): layer = Layer.objects.get(pk=layer.pk) external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.OpenLabor' + external.interoperability = 'nodeshot.interoperability.synchronizers.OpenLabor.OpenLabor' external.config = json.dumps({ "open311_url": '%s/' % TEST_FILES_PATH, "service_code_get": "001", @@ -782,7 +782,7 @@ def test_openlabor_add_node(self): url = 'http://devopenlabor.lynxlab.com/api/v1' external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.OpenLabor' + external.interoperability = 'nodeshot.interoperability.synchronizers.OpenLabor.OpenLabor' external.config = json.dumps({ "open311_url": url, "service_code_get": "001", @@ -820,7 +820,7 @@ def test_nodeshot_sync(self): layer = Layer.objects.get(pk=layer.pk) external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot' + external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot.Nodeshot' external.config = json.dumps({ "layer_url": "https://test.map.ninux.org/api/v1/layers/sicilia/", "verify_ssl": False @@ -901,7 +901,7 @@ def test_nodeshot_sync_exceptions(self): 'http://idonotexi.st.com/hey', 'https://test.map.ninux.org/api/v1/layers/' ]: - external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot' + external.interoperability = 'nodeshot.interoperability.synchronizers.Nodeshot.Nodeshot' external.config = json.dumps({ "layer_url": layer_url, "verify_ssl": False @@ -933,7 +933,7 @@ def test_openwisp_citysdk_tourism(self): xml_url = '%s/openwisp-georss.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWispCitySdkTourism' + external.interoperability = 'nodeshot.interoperability.synchronizers.OpenWispCitySdkTourism.OpenWispCitySdkTourism' config = CITYSDK_TOURISM_TEST_CONFIG.copy() config.update({ "status": "active", @@ -1013,7 +1013,7 @@ def test_geojson_citysdk_tourism(self): url = '%s/geojson1.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJsonCitySdkTourism' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJsonCitySdkTourism.GeoJsonCitySdkTourism' config = CITYSDK_TOURISM_TEST_CONFIG.copy() config.update({ "status": "active", @@ -1113,7 +1113,7 @@ def test_provinciawifi_citysdk_tourism(self): xml_url = '%s/provincia-wifi.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkTourism' + external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkTourism.ProvinciaWifiCitySdkTourism' config = CITYSDK_TOURISM_TEST_CONFIG.copy() config.update({ "status": "active", @@ -1199,7 +1199,7 @@ def test_geojson_citysdk_mobility(self): url = '%s/geojson1.json' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJsonCitySdkMobility' + external.interoperability = 'nodeshot.interoperability.synchronizers.GeoJsonCitySdkMobility.GeoJsonCitySdkMobility' config = CITYSDK_MOBILITY_TEST_CONFIG.copy() config.update({ "url": url, @@ -1299,7 +1299,7 @@ def test_provinciawifi_citysdk_mobility(self): xml_url = '%s/provincia-wifi.xml' % TEST_FILES_PATH external = LayerExternal(layer=layer) - external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkMobility' + external.interoperability = 'nodeshot.interoperability.synchronizers.ProvinciaWifiCitySdkMobility.ProvinciaWifiCitySdkMobility' config = CITYSDK_MOBILITY_TEST_CONFIG.copy() config.update({ "status": "active",