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
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 @@
3# Copyright 2020 Canonical Ltd.3# Copyright 2020 Canonical Ltd.
4# Licensed under the GPLv3, see LICENCE file for details.4# Licensed under the GPLv3, see LICENCE file for details.
55
6import io
7import logging6import logging
8from ops.charm import CharmBase7from ops.charm import CharmBase
9from ops.main import main8from ops.main import main
10from ops.model import ActiveStatus, MaintenanceStatus9from ops.model import ActiveStatus, MaintenanceStatus
11from pprint import pprint10from pprint import pformat
12from yaml import safe_load11from yaml import safe_load
1312
14logger = logging.getLogger()13logger = logging.getLogger()
@@ -24,28 +23,25 @@ class BindK8sCharm(CharmBase):
24 self.framework.observe(self.on.config_changed, self.on_config_changed)23 self.framework.observe(self.on.config_changed, self.on_config_changed)
2524
26 def _check_for_config_problems(self):25 def _check_for_config_problems(self):
27 """Check for some simple configuration problems and return a26 """Return a string describing any configuration problems (or an empty string if none)."""
28 string describing them, otherwise return an empty string."""27 problems = ''
29 problems = []
3028
31 missing = self._missing_charm_settings()29 missing = self._missing_charm_settings()
32 if missing:30 if missing:
33 problems.append('required setting(s) empty: {}'.format(', '.join(sorted(missing))))31 problems = 'required setting(s) empty: {}'.format(', '.join(sorted(missing)))
3432
35 return '; '.join(filter(None, problems))33 return problems
3634
37 def _missing_charm_settings(self):35 def _missing_charm_settings(self):
38 """Check configuration setting dependencies and return a list of36 """Return a list of missing configuration settings (or an empty list if none)."""
39 missing settings; otherwise return an empty list."""
40 config = self.model.config37 config = self.model.config
41 missing = []
4238
43 missing.extend([setting for setting in REQUIRED_SETTINGS if not config[setting]])39 missing = {setting for setting in REQUIRED_SETTINGS if not config[setting]}
4440
45 if config['bind_image_username'] and not config['bind_image_password']:41 if config['bind_image_username'] and not config['bind_image_password']:
46 missing.append('bind_image_password')42 missing.add('bind_image_password')
4743
48 return sorted(list(set(missing)))44 return missing
4945
50 def on_config_changed(self, event):46 def on_config_changed(self, event):
51 """Check that we're leader, and if so, set up the pod."""47 """Check that we're leader, and if so, set up the pod."""
@@ -71,9 +67,7 @@ class BindK8sCharm(CharmBase):
71 """Compile and return our pod resources (e.g. ingresses)."""67 """Compile and return our pod resources (e.g. ingresses)."""
72 # LP#1889746: We need to define a manual ingress here to work around LP#1889703.68 # LP#1889746: We need to define a manual ingress here to work around LP#1889703.
73 resources = {} # TODO69 resources = {} # TODO
74 out = io.StringIO()70 logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(pformat(resources)))
75 pprint(resources, out)
76 logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(out.getvalue()))
77 return resources71 return resources
7872
79 def generate_pod_config(self, secured=True):73 def generate_pod_config(self, secured=True):
@@ -130,9 +124,7 @@ class BindK8sCharm(CharmBase):
130 ],124 ],
131 }125 }
132126
133 out = io.StringIO()127 logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(pformat(spec)))
134 pprint(spec, out)
135 logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
136128
137 if config.get("bind_image_username") and config.get("bind_image_password"):129 if config.get("bind_image_username") and config.get("bind_image_password"):
138 spec.get("containers")[0].get("imageDetails")["username"] = config["bind_image_username"]130 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 =
42 .git,42 .git,
43 __pycache__,43 __pycache__,
44 .tox,44 .tox,
45# Ignore E231 because using black creates errors with this
46ignore = E231
47max-line-length = 12045max-line-length = 120
48max-complexity = 1046max-complexity = 10

Subscribers

People subscribed via source and target branches

to all changes: