[chores] Requested changes

openwisp_monitoring/check/classes/base.py

@@ -2,6 +2,8 @@
 from django.core.exceptions import ValidationError
 from swapper import load_model
 
+from .. import settings as app_settings
+
 Check = load_model('check', 'Check')
 Metric = load_model('monitoring', 'Metric')
 Device = load_model('config', 'Device')
@@ -49,3 +51,13 @@ def _get_or_create_metric(self, configuration=None):
         )
         metric, created = Metric._get_or_create(**options)
         return metric, created
+
+    def _get_ip(self):
+        """
+        Figures out ip to use or fails raising OperationalError
+        """
+        device = self.related_object
+        ip = device.management_ip
+        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
+            ip = device.last_ip
+        return ip

openwisp_monitoring/check/classes/ping.py

@@ -6,7 +6,6 @@
 from swapper import load_model
 
 from ... import settings as monitoring_settings
-from .. import settings as app_settings
 from ..exceptions import OperationalError
 from .base import BaseCheck
 
@@ -123,16 +122,6 @@ def _get_param(self, param):
         """
         return self.params.get(param, self.schema['properties'][param]['default'])
 
-    def _get_ip(self):
-        """
-        Figures out ip to use or fails raising OperationalError
-        """
-        device = self.related_object
-        ip = device.management_ip
-        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
-            ip = device.last_ip
-        return ip
-
     def _command(self, command):
         """
         Executes command (easier to mock)

openwisp_monitoring/check/classes/snmp_devicemonitoring.py

@@ -6,7 +6,6 @@
 
 from openwisp_monitoring.device.api.views import MetricChartsMixin
 
-from .. import settings as app_settings
 from .base import BaseCheck
 
 Chart = load_model('monitoring', 'Chart')
@@ -19,7 +18,7 @@
 class SnmpDeviceMonitoring(BaseCheck, MetricChartsMixin):
     def check(self, store=True):
         result = self.netengine_instance.to_dict()
-        self._init_previous_data()
+        self._init_previous_data(data=getattr(self.related_object, 'data', {}))
         self.related_object.data = result
         if store:
             self.store_result(result)
@@ -35,38 +34,11 @@ def store_result(self, data):
     @cached_property
     def netengine_instance(self):
         ip = self._get_ip()
-        return OpenWRT(host=ip, **self.credential_params)
+        return OpenWRT(host=ip, **self._get_credential_params())
 
-    @cached_property
-    def credential_params(self):
-        params = {}
+    def _get_credential_params(self):
         cred = Credentials.objects.filter(
             deviceconnection__device_id=self.related_object,
             connector='openwisp_controller.connection.connectors.snmp.Snmp',
         ).last()
-        if cred is not None:
-            params.update(cred.params)
-        return params
-
-    def _get_ip(self):
-        """
-        Figures out ip to use or fails raising OperationalError
-        """
-        device = self.related_object
-        ip = device.management_ip
-        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
-            ip = device.last_ip
-        return ip
-
-    def _init_previous_data(self):
-        """
-        makes NetJSON interfaces of previous
-        snapshots more easy to access
-        """
-        data = getattr(self.related_object, 'data', {})
-        if data:
-            data = deepcopy(data)
-            data['interfaces_dict'] = {}
-        for interface in data.get('interfaces', []):
-            data['interfaces_dict'][interface['name']] = interface
-        self._previous_data = data
+        return getattr(cred, 'params', {})

openwisp_monitoring/check/tests/test_ping.py

@@ -227,7 +227,7 @@ def test_auto_chart_disabled(self, *args):
         device = self._create_device(organization=self._create_org())
         device.last_ip = '127.0.0.1'
         device.save()
-        check = Check.objects.first()
+        check = Check.objects.filter(check=self._PING).first()
         self.assertEqual(Chart.objects.count(), 0)
         check.perform_check()
         self.assertEqual(Chart.objects.count(), 0)

openwisp_monitoring/device/api/views.py

@@ -47,12 +47,10 @@ def _write(self, pk, data=None):
         """
         write metrics to database
         """
-        try:
+        if data is None:
             # saves raw device data
             self.instance.save_data()
             data = self.instance.data
-        except AttributeError:
-            pass
         ct = ContentType.objects.get_for_model(Device)
         for interface in data.get('interfaces', []):
             ifname = interface['name']
@@ -317,6 +315,20 @@ def _create_access_tech_chart(self, metric):
         chart.full_clean()
         chart.save()
 
+    def _init_previous_data(self, data=None):
+        """
+        makes NetJSON interfaces of previous
+        snapshots more easy to access
+        """
+        if data is None:
+            data = self.instance.data or {}
+        if data:
+            data = deepcopy(data)
+            data['interfaces_dict'] = {}
+        for interface in data.get('interfaces', []):
+            data['interfaces_dict'][interface['name']] = interface
+        self._previous_data = data
+
 
 class DeviceMetricView(GenericAPIView, MetricChartsMixin):
     model = DeviceData
@@ -448,19 +460,6 @@ def post(self, request, pk):
         )
         return Response(None)
 
-    def _init_previous_data(self):
-        """
-        makes NetJSON interfaces of previous
-        snapshots more easy to access
-        """
-        data = self.instance.data or {}
-        if data:
-            data = deepcopy(data)
-            data['interfaces_dict'] = {}
-        for interface in data.get('interfaces', []):
-            data['interfaces_dict'][interface['name']] = interface
-        self._previous_data = data
-
 
 device_metric = DeviceMetricView.as_view()