Merge ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:charmcraft-review into charm-k8s-bind:master

Proposed by Barry Price
Status: Merged
Approved by: Tom Haddon
Approved revision: b9911ff0859167922fe642933bf9bdcdb6ac9926
Merged at revision: f03ccdfd9c588cb2dac9f1a26e2cbb4a4bafd34f
Proposed branch: ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:charmcraft-review
Merge into: charm-k8s-bind:master
Diff against target: 89 lines (+11/-21)
2 files modified
src/charm.py (+11/-19)
tox.ini (+0/-2)
Reviewer Review Type Date Requested Status
Tom Haddon Approve
Canonical IS Reviewers Pending
Review via email: mp+391966@code.launchpad.net

Commit message

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Tom Haddon (mthaddon) wrote :

LGTM, thanks.

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision f03ccdfd9c588cb2dac9f1a26e2cbb4a4bafd34f

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/charm.py b/src/charm.py
2index 913f69f..0faf651 100755
3--- a/src/charm.py
4+++ b/src/charm.py
5@@ -3,12 +3,11 @@
6 # Copyright 2020 Canonical Ltd.
7 # Licensed under the GPLv3, see LICENCE file for details.
8
9-import io
10 import logging
11 from ops.charm import CharmBase
12 from ops.main import main
13 from ops.model import ActiveStatus, MaintenanceStatus
14-from pprint import pprint
15+from pprint import pformat
16 from yaml import safe_load
17
18 logger = logging.getLogger()
19@@ -24,28 +23,25 @@ class BindK8sCharm(CharmBase):
20 self.framework.observe(self.on.config_changed, self.on_config_changed)
21
22 def _check_for_config_problems(self):
23- """Check for some simple configuration problems and return a
24- string describing them, otherwise return an empty string."""
25- problems = []
26+ """Return a string describing any configuration problems (or an empty string if none)."""
27+ problems = ''
28
29 missing = self._missing_charm_settings()
30 if missing:
31- problems.append('required setting(s) empty: {}'.format(', '.join(sorted(missing))))
32+ problems = 'required setting(s) empty: {}'.format(', '.join(sorted(missing)))
33
34- return '; '.join(filter(None, problems))
35+ return problems
36
37 def _missing_charm_settings(self):
38- """Check configuration setting dependencies and return a list of
39- missing settings; otherwise return an empty list."""
40+ """Return a list of missing configuration settings (or an empty list if none)."""
41 config = self.model.config
42- missing = []
43
44- missing.extend([setting for setting in REQUIRED_SETTINGS if not config[setting]])
45+ missing = {setting for setting in REQUIRED_SETTINGS if not config[setting]}
46
47 if config['bind_image_username'] and not config['bind_image_password']:
48- missing.append('bind_image_password')
49+ missing.add('bind_image_password')
50
51- return sorted(list(set(missing)))
52+ return missing
53
54 def on_config_changed(self, event):
55 """Check that we're leader, and if so, set up the pod."""
56@@ -71,9 +67,7 @@ class BindK8sCharm(CharmBase):
57 """Compile and return our pod resources (e.g. ingresses)."""
58 # LP#1889746: We need to define a manual ingress here to work around LP#1889703.
59 resources = {} # TODO
60- out = io.StringIO()
61- pprint(resources, out)
62- logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(out.getvalue()))
63+ logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(pformat(resources)))
64 return resources
65
66 def generate_pod_config(self, secured=True):
67@@ -130,9 +124,7 @@ class BindK8sCharm(CharmBase):
68 ],
69 }
70
71- out = io.StringIO()
72- pprint(spec, out)
73- logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
74+ logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(pformat(spec)))
75
76 if config.get("bind_image_username") and config.get("bind_image_password"):
77 spec.get("containers")[0].get("imageDetails")["username"] = config["bind_image_username"]
78diff --git a/tox.ini b/tox.ini
79index 91adecf..0de5149 100644
80--- a/tox.ini
81+++ b/tox.ini
82@@ -42,7 +42,5 @@ exclude =
83 .git,
84 __pycache__,
85 .tox,
86-# Ignore E231 because using black creates errors with this
87-ignore = E231
88 max-line-length = 120
89 max-complexity = 10

Subscribers

People subscribed via source and target branches

to all changes: