Merge ~jamesbeedy/charm-grafana:relation_base_to_endpoint into charm-grafana:master

[james beedy]

Fixes https://bugs.launchpad.net/charm-grafana/+bug/1899655

Accompanying changes to interface-grafana-source changes (https://code.launchpad.net/~jamesbeedy/interface-grafana-source/+git/interface-grafana-source/+ref/relation_base_to_endpoint) in respect to replacing RelationBase with Endpoint in the grafana-source requires.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Paul Goins (vultaire)]

Did an initial review.

It looks sane. I'm not sure I've personally dealt with conversations vs. endpoints, but the change makes sense.

I'm not yet ready to +1, as I think some hands-on checking and testing is appropriate here. Further, there's the related changes to interface-grafana-source (https://code.launchpad.net/~jamesbeedy/interface-grafana-source/+git/interface-grafana-source/+merge/393217) which I'm guessing is related here - I haven't looked deeply enough quite to know for sure.

To be clear, this isn't waiting on the author; this is waiting for reviewers (maybe me) to dig into the above points and make sure this won't break.

[Drew Freiberger (afreiberger)]

inline comment below. I think there's an issue with either implementation's logic when we migrate prometheus/influxdb/other grafana-source units but keep the relation.

Either way, I think we have an issue and we'd have to break and re-create the relation if datasources move, if I'm reading the surrounding code accurately.

Agree, this needs some real world testing. I'll address this tomorrow.

[Drew Freiberger (afreiberger)]

I built this branch along with the interface update and it fails grafana-source-relation-departed due to lack of relation.application_name. when removing one of two prometheus units.

unit-grafana-0: 11:53:51 INFO unit.grafana/0.juju-log grafana-source:3: Reactive main running for hook grafana-source-relation-departed
unit-grafana-0: 11:53:51 INFO unit.grafana/0.juju-log grafana-source:3: Initializing Snap Layer
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed All snaps up to date.
unit-grafana-0: 11:53:51 INFO unit.grafana/0.juju-log grafana-source:3: Invoking reactive handler: reactive/grafana.py:609:sources_gone
unit-grafana-0: 11:53:51 INFO unit.grafana/0.juju-log grafana-source:3: sources gone
unit-grafana-0: 11:53:51 INFO unit.grafana/0.juju-log grafana-source:3: <relations.grafana-source.requires.GrafanaRequires object at 0x7fcb186afc88>
unit-grafana-0: 11:53:51 ERROR unit.grafana/0.juju-log grafana-source:3: Hook error:
Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/__init__.py", line 74, in main
    bus.dispatch(restricted=restricted_mode)
  File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/bus.py", line 379, in dispatch
    _invoke(hook_handlers)
  File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/bus.py", line 359, in _invoke
    handler.invoke()
  File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/bus.py", line 181, in invoke
    self._action(*args)
  File "/var/lib/juju/agents/unit-grafana-0/charm/reactive/grafana.py", line 625, in sources_gone
    "service_name": relation.application_name,
AttributeError: 'GrafanaRequires' object has no attribute 'application_name'

unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed Traceback (most recent call last):
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed File "/var/lib/juju/agents/unit-grafana-0/charm/hooks/grafana-source-relation-departed", line 22, in <module>
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed main()
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/__init__.py", line 74, in main
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed bus.dispatch(restricted=restricted_mode)
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/bus.py", line 379, in dispatch
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed _invoke(hook_handlers)
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed File "/var/lib/juju/agents/unit-grafana-0/.venv/lib/python3.6/site-packages/charms/reactive/bus.py", line 359, in _invoke
unit-grafana-0: 11:53:51 WARNING unit.grafana/0.grafana-source-relation-departed handler.invoke()
...

review: Needs Fixing

Unmerged commits

4491ac8... by james beedy on 2020-11-03

refactor sources_gone to accommodate for Endpoint relation

diff --git a/src/reactive/grafana.py b/src/reactive/grafana.py
index d6718eb..a95b271 100644
--- a/src/reactive/grafana.py
+++ b/src/reactive/grafana.py
@@ -616,25 +616,31 @@ def sources_gone(relation):
     hookenv.log(relation)
     # Update the datasources now with the new information that something is
     # not available.
-    conv = relation.conversation()
-    ds = {
-        "service_name": conv.scope.split("/")[0],
-        "type": conv.get_remote("type"),
-        "url": conv.get_remote("url"),
-        "description": conv.get_remote("description"),
-    }
-    hookenv.log("Removing datasource:")
-    hookenv.log(ds)
+    units = relation.all_joined_units
+    if len(units) > 0:
+
+        unit = units[0]
+
+        ds = {
+            "service_name": relation.application_name,
+            "type": unit.received.get('type'),
+            "url": unit.received.get('url'),
+            "description": unit.received.get('description'),
+        }
+
+        hookenv.log("Removing datasource:")
+        hookenv.log(ds)
+
+        if ds:
+            conn = sqlite3.connect("{}/grafana.db".format(data_dir), timeout=30)
+            cur = conn.cursor()
+            cur.execute(
+                "DELETE FROM DATA_SOURCE WHERE  type=? AND url=?",
+                (ds["type"], ds["url"]),
+            )
+            conn.commit()
+            cur.close()
 
-    if ds:
-        conn = sqlite3.connect("{}/grafana.db".format(data_dir), timeout=30)
-        cur = conn.cursor()
-        cur.execute(
-            "DELETE FROM DATA_SOURCE WHERE  type=? AND url=?",
-            (ds["type"], ds["url"]),
-        )
-        conn.commit()
-        cur.close()
     relation.set_local = None