bind-charmers team mailing list archive

[Barry Price <barry.price@xxxxxxxxxxxxx>]

Barry Price has proposed merging ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:charmcraft-review into charm-k8s-bind:master.

Commit message:
This is the branch approved by charmcraft in this MP:

https://code.launchpad.net/~bind-charmers/charm-k8s-bind/+git/charmcraft-review/+merge/389995

Requested reviews:
  Bind Charmers (bind-charmers)

For more details, see:
https://code.launchpad.net/~barryprice/charm-k8s-bind/+git/charm-k8s-bind/+merge/391966
-- 
Your team Bind Charmers is requested to review the proposed merge of ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:charmcraft-review into charm-k8s-bind:master.

diff --git a/src/charm.py b/src/charm.py
index 913f69f..0faf651 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -3,12 +3,11 @@
 # Copyright 2020 Canonical Ltd.
 # Licensed under the GPLv3, see LICENCE file for details.
 
-import io
 import logging
 from ops.charm import CharmBase
 from ops.main import main
 from ops.model import ActiveStatus, MaintenanceStatus
-from pprint import pprint
+from pprint import pformat
 from yaml import safe_load
 
 logger = logging.getLogger()
@@ -24,28 +23,25 @@ class BindK8sCharm(CharmBase):
         self.framework.observe(self.on.config_changed, self.on_config_changed)
 
     def _check_for_config_problems(self):
-        """Check for some simple configuration problems and return a
-        string describing them, otherwise return an empty string."""
-        problems = []
+        """Return a string describing any configuration problems (or an empty string if none)."""
+        problems = ''
 
         missing = self._missing_charm_settings()
         if missing:
-            problems.append('required setting(s) empty: {}'.format(', '.join(sorted(missing))))
+            problems = 'required setting(s) empty: {}'.format(', '.join(sorted(missing)))
 
-        return '; '.join(filter(None, problems))
+        return problems
 
     def _missing_charm_settings(self):
-        """Check configuration setting dependencies and return a list of
-        missing settings; otherwise return an empty list."""
+        """Return a list of missing configuration settings (or an empty list if none)."""
         config = self.model.config
-        missing = []
 
-        missing.extend([setting for setting in REQUIRED_SETTINGS if not config[setting]])
+        missing = {setting for setting in REQUIRED_SETTINGS if not config[setting]}
 
         if config['bind_image_username'] and not config['bind_image_password']:
-            missing.append('bind_image_password')
+            missing.add('bind_image_password')
 
-        return sorted(list(set(missing)))
+        return missing
 
     def on_config_changed(self, event):
         """Check that we're leader, and if so, set up the pod."""
@@ -71,9 +67,7 @@ class BindK8sCharm(CharmBase):
         """Compile and return our pod resources (e.g. ingresses)."""
         # LP#1889746: We need to define a manual ingress here to work around LP#1889703.
         resources = {}  # TODO
-        out = io.StringIO()
-        pprint(resources, out)
-        logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(out.getvalue()))
+        logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(pformat(resources)))
         return resources
 
     def generate_pod_config(self, secured=True):
@@ -130,9 +124,7 @@ class BindK8sCharm(CharmBase):
             ],
         }
 
-        out = io.StringIO()
-        pprint(spec, out)
-        logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
+        logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(pformat(spec)))
 
         if config.get("bind_image_username") and config.get("bind_image_password"):
             spec.get("containers")[0].get("imageDetails")["username"] = config["bind_image_username"]
diff --git a/tox.ini b/tox.ini
index 91adecf..0de5149 100644
--- a/tox.ini
+++ b/tox.ini
@@ -42,7 +42,5 @@ exclude =
     .git,
     __pycache__,
     .tox,
-# Ignore E231 because using black creates errors with this
-ignore = E231
 max-line-length = 120
 max-complexity = 10