Merge lp:~dpb/juju-deployer/empty-options-1361883 into lp:juju-deployer

Proposed by David Britton
Status: Merged
Merged at revision: 121
Proposed branch: lp:~dpb/juju-deployer/empty-options-1361883
Merge into: lp:juju-deployer
Diff against target: 72 lines (+35/-1)
5 files modified
Makefile (+2/-0)
deployer/deployment.py (+4/-1)
deployer/tests/test_data/negative.cfg (+10/-0)
deployer/tests/test_data/negative.yaml (+5/-0)
deployer/tests/test_deployment.py (+14/-0)
To merge this branch: bzr merge lp:~dpb/juju-deployer/empty-options-1361883
Reviewer Review Type Date Requested Status
Kapil Thangavelu Approve
Review via email: mp+232332@code.launchpad.net

Description of the change

Simple none type check to fix. test cases fail if you revert the patch to deployment.py

To post a comment you must log in.
Revision history for this message
Kapil Thangavelu (hazmat) wrote :

thanks, fwiw for test running i typically just do python setup.py test

Revision history for this message
Kapil Thangavelu (hazmat) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2014-02-22 23:33:57 +0000
3+++ Makefile 2014-08-26 22:36:31 +0000
4@@ -1,3 +1,5 @@
5+test:
6+ nosetests -s --verbosity=2 deployer/tests
7
8 freeze:
9 pip install -d tools/dist -r requirements.txt
10
11=== modified file 'deployer/deployment.py'
12--- deployer/deployment.py 2014-04-21 22:49:05 +0000
13+++ deployer/deployment.py 2014-08-26 22:36:31 +0000
14@@ -183,7 +183,10 @@
15 config = charm.config
16 options = {}
17
18- for k, v in svc_data['options'].items():
19+ svc_options = svc_data.get('options', {})
20+ if svc_options is None:
21+ svc_options = {}
22+ for k, v in svc_options.items():
23 if not k in config:
24 feedback.error(
25 "Invalid config charm %s %s=%s" % (charm.name, k, v))
26
27=== added file 'deployer/tests/test_data/negative.cfg'
28--- deployer/tests/test_data/negative.cfg 1970-01-01 00:00:00 +0000
29+++ deployer/tests/test_data/negative.cfg 2014-08-26 22:36:31 +0000
30@@ -0,0 +1,10 @@
31+{
32+ "negative": {
33+ "services": {
34+ "foo": {
35+ # lp:1361883
36+ "options": {}
37+ }
38+ }
39+ }
40+}
41
42=== added file 'deployer/tests/test_data/negative.yaml'
43--- deployer/tests/test_data/negative.yaml 1970-01-01 00:00:00 +0000
44+++ deployer/tests/test_data/negative.yaml 2014-08-26 22:36:31 +0000
45@@ -0,0 +1,5 @@
46+negative:
47+ services:
48+ foo:
49+ options:
50+ # lp:1361883
51
52=== modified file 'deployer/tests/test_deployment.py'
53--- deployer/tests/test_deployment.py 2014-02-18 14:33:13 +0000
54+++ deployer/tests/test_deployment.py 2014-08-26 22:36:31 +0000
55@@ -156,3 +156,17 @@
56 expected_service_names = [
57 'ceph', 'mysql', 'nova-compute', 'quantum', 'semper', 'verity']
58 self.assertEqual(set(expected_service_names), set(service_names))
59+
60+ def test_resolve_config_handles_empty_options(self):
61+ """resolve_config should handle options being "empty" lp:1361883"""
62+ deployment = self.get_named_deployment("negative.cfg", "negative")
63+ self.assertEqual(
64+ deployment.data["services"]["foo"]["options"], {})
65+ deployment.resolve_config()
66+
67+ def test_resolve_config_handles_none_options(self):
68+ """resolve_config should handle options being "none" lp:1361883"""
69+ deployment = self.get_named_deployment("negative.yaml", "negative")
70+ self.assertEqual(
71+ deployment.data["services"]["foo"]["options"], None)
72+ deployment.resolve_config()

Subscribers

People subscribed via source and target branches