Merge lp:~adam-collard/charms/trusty/apache2/apache2-charmhelpers-fix into lp:charms/trusty/apache2

Proposed by Adam Collard on 2015-03-10
Status: Merged
Approved by: David Britton on 2015-03-10
Approved revision: 64
Merged at revision: 64
Proposed branch: lp:~adam-collard/charms/trusty/apache2/apache2-charmhelpers-fix
Merge into: lp:charms/trusty/apache2
Diff against target: 33 lines (+14/-1)
2 files modified
hooks/hooks.py (+1/-1)
hooks/tests/test_config_changed.py (+13/-0)
To merge this branch: bzr merge lp:~adam-collard/charms/trusty/apache2/apache2-charmhelpers-fix
Reviewer Review Type Date Requested Status
David Britton (community) 2015-03-10 Approve on 2015-03-10
Review via email: mp+252497@code.launchpad.net

Description of the change

Fix the workaround in the Apache2 charm for an empty servername.

To post a comment you must log in.
David Britton (davidpbritton) wrote :

Tested with landscape-dense-maas bundle (where problem was initially spotted), all is fine, verified unit test coverage.

Thanks for the change! looks great!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py 2015-02-27 14:22:12 +0000
+++ hooks/hooks.py 2015-03-10 19:17:23 +0000
@@ -283,7 +283,7 @@
283 result = orig_config_get(scope)283 result = orig_config_get(scope)
284 if scope == "servername" and len(result) == 0:284 if scope == "servername" and len(result) == 0:
285 result = unit_get("public-address")285 result = unit_get("public-address")
286 elif type(result) is dict and result.get("servername", "") == "":286 elif isinstance(result, dict) and result.get("servername", "") == "":
287 result["servername"] = unit_get("public-address")287 result["servername"] = unit_get("public-address")
288 return result288 return result
289289
290290
=== modified file 'hooks/tests/test_config_changed.py'
--- hooks/tests/test_config_changed.py 2014-12-29 22:57:24 +0000
+++ hooks/tests/test_config_changed.py 2015-03-10 19:17:23 +0000
@@ -74,3 +74,16 @@
74 len(os.listdir("%s/%s" % (self.dirname, "sites-enabled"))), 0)74 len(os.listdir("%s/%s" % (self.dirname, "sites-enabled"))), 0)
75 self.assertEqual(75 self.assertEqual(
76 len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 0)76 len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 0)
77
78 @patch('hooks.orig_config_get')
79 @patch('hooks.unit_get')
80 def test_config_get_works_with_dict_subclass(
81 self, mock_unit_get, mock_orig_config_get):
82 """config_get() works with Charm Helper's custom dict subclass"""
83 class FakeConfig(dict):
84 pass
85
86 mock_orig_config_get.return_value = FakeConfig(servername="")
87 mock_unit_get.return_value = "foo"
88 config = hooks.config_get()
89 self.assertEqual(config["servername"], "foo")

Subscribers

People subscribed via source and target branches

to all changes: