From 34a196a4720ae0179d76e932d061e701356f1c26 Mon Sep 17 00:00:00 2001 From: purhan Date: Thu, 5 Aug 2021 23:01:44 +0530 Subject: [PATCH] [chores] Requested changes --- openwisp_monitoring/check/classes/base.py | 12 +++++++ openwisp_monitoring/check/classes/ping.py | 11 ------ .../check/classes/snmp_devicemonitoring.py | 36 +++---------------- openwisp_monitoring/check/tests/test_ping.py | 2 +- openwisp_monitoring/device/api/views.py | 31 ++++++++-------- 5 files changed, 32 insertions(+), 60 deletions(-) diff --git a/openwisp_monitoring/check/classes/base.py b/openwisp_monitoring/check/classes/base.py index c24b4c7ce..1f4967556 100644 --- a/openwisp_monitoring/check/classes/base.py +++ b/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 diff --git a/openwisp_monitoring/check/classes/ping.py b/openwisp_monitoring/check/classes/ping.py index 960b78a71..da547888f 100644 --- a/openwisp_monitoring/check/classes/ping.py +++ b/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) diff --git a/openwisp_monitoring/check/classes/snmp_devicemonitoring.py b/openwisp_monitoring/check/classes/snmp_devicemonitoring.py index da8f24c10..75fa6c332 100644 --- a/openwisp_monitoring/check/classes/snmp_devicemonitoring.py +++ b/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', {}) diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index ff1650f1a..c635a9d67 100644 --- a/openwisp_monitoring/check/tests/test_ping.py +++ b/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) diff --git a/openwisp_monitoring/device/api/views.py b/openwisp_monitoring/device/api/views.py index 6392d8a18..3e3864931 100644 --- a/openwisp_monitoring/device/api/views.py +++ b/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()