Merge ~barryprice/charm-k8s-discourse/+git/charm-k8s-discourse:v2.8.0.beta7-20.04 into charm-k8s-discourse:v2.8.0.beta7-20.04

[Barry Price]

Run charmcraft fetch-lib to update our redis_k8s library, and update tox.ini to use the correct PYTHONPATH

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

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

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

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

FAILED: Continuous integration, rev:28e3d00a39acdfbbde92cc02e8ce5703fab19595
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/28/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/is/job/lp-charm-test/143/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/515054/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/28//rebuild

review: Needs Fixing (continuous-integration)

[Tom Haddon (mthaddon)]

A comment inline about the path being controllable via tox.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[Barry Price (barryprice)]

Hmm I think I'm missing something then, tox.ini already has:

[testenv]
basepython = python3
setenv =
  PYTHONPATH = {toxinidir}/build/lib:{toxinidir}/build/venv

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

FAILED: Continuous integration, rev:f06849d682c54ded134966539be23e3bc30414b4
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/30/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/is/job/lp-charm-test/145/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/523855/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/30//rebuild

review: Needs Fixing (continuous-integration)

[Tom Haddon (mthaddon)]

I think there's been a change in charmcraft which means the "build" directory is no longer used as before (it's now used in a way similar to snapcraft).

As a result I think you'll want something like this https://pastebin.ubuntu.com/p/kkx5d6Pgf2/ - essentially just adding `{toxinidir}/lib` to the testenv path. Fwiw, Jon Seager's teams will be standardising on a tox.ini similar to this https://github.com/jnsgruk/charm-kubernetes-dashboard/blob/main/tox.ini. I'm not saying we should change to this exactly, but it shows `{toxinidir}/lib` as part of testenv.

[Barry Price (barryprice)]

Thanks, that works locally - have pushed another revision.

Removing the existing directories in PYTHONPATH doesn't seem to cause any problems either, so if those are historical I think they can go too.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

FAILED: Continuous integration, rev:902dfd48f34c356618ef9da905fb2ca9ec4a119f
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/31/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/is/job/lp-charm-test/146/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/524632/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-discourse-ci/31//rebuild

review: Needs Fixing (continuous-integration)

[Tom Haddon (mthaddon)]

LGTM, thx

review: Approve

[Barry Price (barryprice)]

Mergebot can't land this because of LP:1949763 and LP:1949764.

Marking it WIP until those are addressed, then we can use this to test the fixes.

diff --git a/lib/charms/redis_k8s/v0/redis.py b/lib/charms/redis_k8s/v0/redis.py
index 70787b6..5360ce0 100644
--- a/lib/charms/redis_k8s/v0/redis.py
+++ b/lib/charms/redis_k8s/v0/redis.py
@@ -6,15 +6,29 @@ redis interface.
 Import `RedisRequires` in your charm by adding the following to `src/charm.py`:
 ```
 from charms.redis_k8s.v0.redis import RedisRequires
-
-# In your charm's `__init__` method.
+```
+And then in your charm's `__init__` method:
+```
+# Make sure you set redis_relation in StoredState. Assuming you refer to this
+# as `self._stored`:
+self._stored.set_default(redis_relation={})
 self.redis = RedisRequires(self, self._stored)
 ```
-And then wherever you need to reference the relation data:
+And then wherever you need to reference the relation data it will be available
+via StoredState:
 ```
+redis_hosts = []
 for redis_unit in self._stored.redis_relation:
-    redis_host = self._stored.redis_relation[redis_unit]["hostname"]
-    redis_port = self._stored.redis_relation[redis_unit]["port"]
+    redis_hosts.append({
+        "redis-host": self._stored.redis_relation[redis_unit]["hostname"],
+        "redis-port": self._stored.redis_relation[redis_unit]["port"],
+    })
+```
+You will also need to add the following to `metadata.yaml`:
+```
+requires:
+  redis:
+    interface: redis
 ```
 """
 import logging
@@ -22,30 +36,31 @@ import logging
 from ops.charm import CharmEvents
 from ops.framework import EventBase, EventSource, Object
 
-# The unique Charmhub library identifier, never change it
+# The unique Charmhub library identifier, never change it.
 LIBID = "fe18a608cec5465fa5153e419abcad7b"
 
-# Increment this major API version when introducing breaking changes
+# Increment this major API version when introducing breaking changes.
 LIBAPI = 0
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
-# to 0 if you are raising the major API version
-LIBPATCH = 1
+# to 0 if you are raising the major API version.
+LIBPATCH = 2
 
 logger = logging.getLogger(__name__)
 
 
 class RedisRelationUpdatedEvent(EventBase):
-    pass
+    """An event for the redis relation having been updated."""
 
 
 class RedisRelationCharmEvents(CharmEvents):
+    """A class to carry custom charm events so requires can react to relation changes."""
     redis_relation_updated = EventSource(RedisRelationUpdatedEvent)
 
 
 class RedisRequires(Object):
     def __init__(self, charm, _stored):
-        # Define a constructor that takes the charm and it's StoredState
+        """A class implementing the redis requires relation."""
         super().__init__(charm, "redis")
         self.framework.observe(charm.on.redis_relation_changed, self._on_relation_changed)
         self.framework.observe(charm.on.redis_relation_broken, self._on_relation_broken)
@@ -53,19 +68,20 @@ class RedisRequires(Object):
         self.charm = charm
 
     def _on_relation_changed(self, event):
+        """Handle the relation changed event."""
         if not event.unit:
             return
 
         hostname = event.relation.data[event.unit].get("hostname")
         port = event.relation.data[event.unit].get("port")
-        # Store some data from the relation in local state
         self._stored.redis_relation[event.relation.id] = {"hostname": hostname, "port": port}
 
         # Trigger an event that our charm can react to.
         self.charm.on.redis_relation_updated.emit()
 
     def _on_relation_broken(self, event):
-        # Remove the unit data from local state
+        """Handle the relation broken event."""
+        # Remove the unit data from local state.
         self._stored.redis_relation.pop(event.relation.id, None)
 
         # Trigger an event that our charm can react to.
@@ -74,23 +90,26 @@ class RedisRequires(Object):
 
 class RedisProvides(Object):
     def __init__(self, charm, port):
+        """A class implementing the redis provides relation."""
         super().__init__(charm, "redis")
         self.framework.observe(charm.on.redis_relation_changed, self._on_relation_changed)
         self._port = port
 
     def _on_relation_changed(self, event):
+        """Handle the relation changed event."""
         if not self.model.unit.is_leader():
             logger.debug("Relation changes ignored by non-leader")
             return
 
         event.relation.data[self.model.unit]['hostname'] = str(self._bind_address(event))
         event.relation.data[self.model.unit]['port'] = str(self._port)
-        # The reactive Redis charm exposes also 'password'. When tackling
+        # The reactive Redis charm also exposes 'password'. When tackling
         # https://github.com/canonical/redis-operator/issues/7 add 'password'
         # field so that it matches the exposed interface information from it.
         # event.relation.data[self.unit]['password'] = ''
 
     def _bind_address(self, event):
+        """Convenience function for getting the unit address."""
         relation = self.model.get_relation(event.relation.name, event.relation.id)
         if address := self.model.get_binding(relation).network.bind_address:
             return address
diff --git a/tox.ini b/tox.ini
index d48b030..857a545 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,8 +5,6 @@ skip_missing_interpreters = True
 
 [testenv]
 basepython = python3
-setenv =
-  PYTHONPATH = {toxinidir}/build/lib:{toxinidir}/build/venv
 
 [testenv:unit]
 commands =
@@ -15,7 +13,7 @@ commands =
 deps = -r{toxinidir}/tests/unit/requirements.txt
        -r{toxinidir}/requirements.txt
 setenv =
-  PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv
+  PYTHONPATH={toxinidir}/src:{toxinidir}/lib
   TZ=UTC
 
 [testenv:black]
@@ -32,4 +30,3 @@ exclude = .git, __pycache__, .tox,
 ignore = E231
 max-line-length = 120
 max-complexity = 10
-