Merge lp:~jjo/juju-deployer/implement-series-cli-override into lp:juju-deployer

Proposed by JuanJo Ciarlante
Status: Rejected
Rejected by: JuanJo Ciarlante
Proposed branch: lp:~jjo/juju-deployer/implement-series-cli-override
Merge into: lp:juju-deployer
Diff against target: 93 lines (+22/-5)
5 files modified
deployer/action/importer.py (+2/-1)
deployer/cli.py (+4/-0)
deployer/deployment.py (+6/-3)
deployer/tests/test_deployment.py (+9/-1)
deployer/tests/test_guiserver.py (+1/-0)
To merge this branch: bzr merge lp:~jjo/juju-deployer/implement-series-cli-override
Reviewer Review Type Date Requested Status
Kapil Thangavelu Needs Fixing
Review via email: mp+202778@code.launchpad.net

Commit message

[jjo,r=] implement --series= cli override

TESTS ok.

To post a comment you must log in.
85. By Kapil Thangavelu

merge branch mthaddon doc-updates re inheriteance

86. By Kapil Thangavelu

merge jjo branch for offline build server testing on precise

87. By Kapil Thangavelu

merge support for JUJU_REPOSITORY, variant of jjo support-JUJU_REPOSITORY-env_var branch

88. By Martin Packman

merge gz branch for juju binary check and helpful err msg if missing

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

lgtm, although i'd like to drop the shorthand -i and just keep long form --series.

review: Approve
Revision history for this message
JuanJo Ciarlante (jjo) wrote :

> lgtm, although i'd like to drop the shorthand -i and just keep long form
> --series.

Ack, done.

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

there's something a bit strange here which the tests note as part of their workaround, namely the deployment is using the value prior to its override, which makes this feels a bit icky/nondeterministic ie. runtime value changing while being used. the cleaner way would be to override this as the deployment is being constructed in configstack.

review: Needs Fixing
89. By JuanJo Ciarlante

merge jjo diff usability fixes

90. By JuanJo Ciarlante

merge branch jjo diff tests

91. By Kapil Thangavelu

merge hazmat rework error handling to use feedback object to collect sets of validation errors

92. By Brad Crittenden

merge bac parse-constraints, normalize constraints for use with api deploys

93. By JuanJo Ciarlante

implement --series cli override, also fix wiki.yaml parsing

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'deployer/action/importer.py'
--- deployer/action/importer.py 2013-11-12 04:54:58 +0000
+++ deployer/action/importer.py 2014-01-23 11:46:28 +0000
@@ -64,7 +64,8 @@
6464
65 # Load config overrides/includes and verify rels after we can65 # Load config overrides/includes and verify rels after we can
66 # validate them.66 # validate them.
67 self.deployment.resolve(self.options.overrides or ())67 self.deployment.resolve(self.options.overrides or (),
68 self.options.series)
6869
69 def deploy_services(self):70 def deploy_services(self):
70 self.log.info("Deploying services...")71 self.log.info("Deploying services...")
7172
=== modified file 'deployer/cli.py'
--- deployer/cli.py 2013-12-18 09:51:54 +0000
+++ deployer/cli.py 2014-01-23 11:46:28 +0000
@@ -88,6 +88,10 @@
88 'across all services. Input as key=value.'),88 'across all services. Input as key=value.'),
89 dest='overrides', default=None)89 dest='overrides', default=None)
90 parser.add_argument(90 parser.add_argument(
91 '--series', type=str,
92 help=('Override distro series in config file'),
93 dest='series', default=None)
94 parser.add_argument(
91 '-v', '--verbose', action='store_true', default=False,95 '-v', '--verbose', action='store_true', default=False,
92 dest="verbose", help='Verbose output')96 dest="verbose", help='Verbose output')
93 parser.add_argument(97 parser.add_argument(
9498
=== modified file 'deployer/deployment.py'
--- deployer/deployment.py 2013-10-31 19:19:36 +0000
+++ deployer/deployment.py 2014-01-23 11:46:28 +0000
@@ -177,19 +177,22 @@
177 continue177 continue
178 charm.fetch()178 charm.fetch()
179179
180 def resolve(self, cli_overides=()):180 def resolve(self, cli_overides=(), cli_series=None):
181 # Once we have charms definitions available, we can do verification181 # Once we have charms definitions available, we can do verification
182 # of config options.182 # of config options.
183 self.load_overrides(cli_overides)183 self.load_overrides(cli_overides, cli_series)
184 self.resolve_config()184 self.resolve_config()
185 self.validate_relations()185 self.validate_relations()
186 self.validate_placement()186 self.validate_placement()
187187
188 def load_overrides(self, cli_overrides=()):188 def load_overrides(self, cli_overrides=(), cli_series=None):
189 """Load overrides."""189 """Load overrides."""
190 overrides = {}190 overrides = {}
191 overrides.update(self.data.get('overrides', {}))191 overrides.update(self.data.get('overrides', {}))
192192
193 if cli_series:
194 self.data['series'] = cli_series
195
193 for o in cli_overrides:196 for o in cli_overrides:
194 key, value = o.split('=', 1)197 key, value = o.split('=', 1)
195 overrides[key] = value198 overrides[key] = value
196199
=== modified file 'deployer/tests/test_deployment.py'
--- deployer/tests/test_deployment.py 2014-01-23 09:50:55 +0000
+++ deployer/tests/test_deployment.py 2014-01-23 11:46:28 +0000
@@ -43,9 +43,17 @@
43 d.fetch_charms()43 d.fetch_charms()
4444
45 # Load up overrides and resolves45 # Load up overrides and resolves
46 d.load_overrides(["key=abc"])46 # kludge: because this test actually expects series' charms
47 # to be downloadable, override it (back) to 'precise' by
48 # previously patching it to some other value.
49 d.data['series'] = 'zeriez'
50 self.assertEqual(d.series, 'zeriez')
51
52 d.load_overrides(["key=abc"], 'precise')
47 d.resolve_config()53 d.resolve_config()
4854
55 # Verify series override
56 self.assertEqual(d.series, 'precise')
49 # Verify include-base6457 # Verify include-base64
50 self.assertEqual(d.get_service('newrelic').config, {'key': 'abc'})58 self.assertEqual(d.get_service('newrelic').config, {'key': 'abc'})
51 self.assertEqual(59 self.assertEqual(
5260
=== modified file 'deployer/tests/test_guiserver.py'
--- deployer/tests/test_guiserver.py 2014-01-23 09:50:55 +0000
+++ deployer/tests/test_guiserver.py 2014-01-23 11:46:28 +0000
@@ -225,6 +225,7 @@
225 overrides=None,225 overrides=None,
226 rel_wait=60,226 rel_wait=60,
227 retry_count=0,227 retry_count=0,
228 series=None,
228 terminate_machines=False,229 terminate_machines=False,
229 timeout=2700,230 timeout=2700,
230 update_charms=False,231 update_charms=False,

Subscribers

People subscribed via source and target branches