[bug] AttributeError: 'str' object has no attribute 'copy' in deep_merge_dicts

[nemesifier]

@nepython noticed a problem when I tested the latest master in staging which has this in the settings.py:

OPENWISP_MONITORING_CHARTS = {
    'traffic': {
        'unit': ' MB',
        'description': (
            'Network traffic, download and upload, measured on '
            'the interface "{metric.key}", measured in MB.'
        ),
        'query': {
            'influxdb': (
                "SELECT SUM(tx_bytes) / 1000000 AS upload, "
                "SUM(rx_bytes) / 1000000 AS download FROM {key} "
                "WHERE time >= '{time}' AND content_type = '{content_type}' "
                "AND object_id = '{object_id}' GROUP BY time(1d)"
            )
        },
    }
}

I get the following error:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/openwisp2/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/opt/openwisp2/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 377, in execute
    django.setup()
  File "/opt/openwisp2/env/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/opt/openwisp2/env/lib/python3.6/site-packages/django/apps/registry.py", line 114, in populate
    app_config.import_models()
  File "/opt/openwisp2/env/lib/python3.6/site-packages/django/apps/config.py", line 211, in import_models
    self.models_module = import_module(models_module_name)
  File "/opt/openwisp2/env/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/models.py", line 3, in <module>
    from .base.models import AbstractAlertSettings, AbstractChart, AbstractMetric
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/base/models.py", line 195, in <module>
    class AbstractChart(TimeStampedEditableModel):
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/base/models.py", line 196, in AbstractChart
    CHARTS = get_chart_configuration()
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/charts.py", line 153, in get_chart_configuration
    charts = deep_merge_dicts(DEFAULT_CHARTS, app_settings.ADDITIONAL_CHARTS)
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/charts.py", line 146, in deep_merge_dicts
    result[key] = deep_merge_dicts(node, value)
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/charts.py", line 146, in deep_merge_dicts
    result[key] = deep_merge_dicts(node, value)
  File "/opt/openwisp2/env/lib/python3.6/site-packages/openwisp_monitoring/monitoring/charts.py", line 142, in deep_merge_dicts
    result = dict1.copy()
AttributeError: 'str' object has no attribute 'copy'

Please try to replicate it and fix it.

[nemesifier]

Update: after rolling back to https://github.com/openwisp/openwisp-monitoring/commit/0a8b4d8db03c7609277f8c1ff47b71533264c8ee the application starts. I think something in the change which touched deep_merge_dict in #103 went wrong, maybe there's a slight difference between the version in openwisp-utils and the one which was shipped in here?

[nepython]

Actually in the PR, I removed the influxdb key. The reason was simple, since queries shall be different for different dbs, we need to store all of them in s separate dat structure than the chart configuration and import it the same from respective timeseries db. So, after moving the queries to influxdb backend, I removed the influxdb key as it wasn't needed anymore, I think.

OPENWISP_MONITORING_CHARTS = {
    'traffic': {
        'unit': ' MB',
        'description': (
            'Network traffic, download and upload, measured on '
            'the interface "{metric.key}", measured in MB.'
        ),
        'query': {
            (       # difference is here
                "SELECT SUM(tx_bytes) / 1000000 AS upload, "
                "SUM(rx_bytes) / 1000000 AS download FROM {key} "
                "WHERE time >= '{time}' AND content_type = '{content_type}' "
                "AND object_id = '{object_id}' GROUP BY time(1d)"
            )
        },
    }
}

If you do it as above everything shall work fine, so it's not a bug I think but this part can be refactored if needed.

[nemesifier]

@nepython I don't understand:

>>> q = {'query': {
...             (       # difference is here
...                 "SELECT SUM(tx_bytes) / 1000000 AS upload, "
...                 "SUM(rx_bytes) / 1000000 AS download FROM {key} "
...                 "WHERE time >= '{time}' AND content_type = '{content_type}' "
...                 "AND object_id = '{object_id}' GROUP BY time(1d)"
...             )
...         }
... }
>>> q['query']
set(["SELECT SUM(tx_bytes) / 1000000 AS upload, SUM(rx_bytes) / 1000000 AS download FROM {key} WHERE time >= '{time}' AND content_type = '{content_type}' AND object_id = '{object_id}' GROUP BY time(1d)"])
>>> type(q['query'])
<type 'set'>

The query is a set, is that intentional? Why is it a set?

Did you notice that the examples in the README are pointing out to the configuration I posted in the issue description and therefore that documented feature is now broken?

I see several problems here:

• a documented feature was broken, we are introducing a backward incompatible change, which will require all the instances that are using openwisp-monitoring and are using customized queries to upgrade, which is not nice
• an automated test for this use case is missing, we need to add one

The preferred solution is to fix the code so that the current way of doing it keeps working.