Merge lp:~frankban/juju-quickstart/open-urls into lp:juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 9
Proposed branch: lp:~frankban/juju-quickstart/open-urls
Merge into: lp:juju-quickstart
Diff against target: 445 lines (+220/-35)
7 files modified
quickstart/app.py (+12/-4)
quickstart/manage.py (+15/-13)
quickstart/tests/helpers.py (+17/-0)
quickstart/tests/test_app.py (+38/-11)
quickstart/tests/test_manage.py (+32/-7)
quickstart/tests/test_utils.py (+73/-0)
quickstart/utils.py (+33/-0)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/open-urls
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+193297@code.launchpad.net

Description of the change

Deploy a bundle from a HTTP(S) url.

The bundle argument can be a file path or
a http/https URL pointing to a bundle YAML.

Also get the URL of the last Juju GUI charm
revision from charmworld (API 2 for now).

Tests: `make check`.

QA:
I copied a bundle YAML over here:
http://dpaste.com/1435065/plain/
You should be able to deploy that bundle
by running the following (after juju switching to ec2 or similar):
`.venv/bin/python juju-quickstart http://dpaste.com/1435065/plain/`
Remember to destroy your ec2 environment...

https://codereview.appspot.com/19870043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+193297_code.launchpad.net,

Message:
Please take a look.

Description:
Deploy a bundle from a HTTP(S) url.

The bundle argument can be a file path or
a http/https URL pointing to a bundle YAML.

Also get the URL of the last Juju GUI charm
revision from charmworld (API 2 for now).

Tests: `make check`.

QA:
I copied a bundle YAML over here:
http://dpaste.com/1435065/plain/
You should be able to deploy that bundle
by running the following (after juju switching to ec2 or similar):
`.venv/bin/python juju-quickstart http://dpaste.com/1435065/plain/`
Remember to destroy your ec2 environment...

https://code.launchpad.net/~frankban/juju-quickstart/open-urls/+merge/193297

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/19870043/

Affected files (+224, -35 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/tests/helpers.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

Revision history for this message
Gary Poster (gary) wrote :

Code LGTM with trivial update. Trying QA now.

https://codereview.appspot.com/19870043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/19870043/diff/1/quickstart/app.py#newcode126
quickstart/app.py:126: logging.warn(msg.format(err))
nice.

https://codereview.appspot.com/19870043/diff/1/quickstart/tests/test_app.py
File quickstart/tests/test_app.py (right):

https://codereview.appspot.com/19870043/diff/1/quickstart/tests/test_app.py#newcode263
quickstart/tests/test_app.py:263: with
self.patch_get_charm_url(side_effect=IOError('boo!')):
You scared me.

https://codereview.appspot.com/19870043/diff/1/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/19870043/diff/1/quickstart/utils.py#newcode35
quickstart/utils.py:35: # Switch to charmworld API 3 when the 500 error
is fixed.
We had a fix and a deployment, and
http://manage.jujucharms.com/api/3/charm/precise/juju-gui works now
(yay!)

https://codereview.appspot.com/19870043/

Revision history for this message
Gary Poster (gary) wrote :

Go you and your bad 100% test coverage self.

qa good (though EC2 left a bunch of services in pending state and the
haproxy and daisy charms were DOA :-/ )

Thank you!

https://codereview.appspot.com/19870043/

11. By Francesco Banconi

Changes as per review.

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Deploy a bundle from a HTTP(S) url.

The bundle argument can be a file path or
a http/https URL pointing to a bundle YAML.

Also get the URL of the last Juju GUI charm
revision from charmworld (API 2 for now).

Tests: `make check`.

QA:
I copied a bundle YAML over here:
http://dpaste.com/1435065/plain/
You should be able to deploy that bundle
by running the following (after juju switching to ec2 or similar):
`.venv/bin/python juju-quickstart http://dpaste.com/1435065/plain/`
Remember to destroy your ec2 environment...

R=gary.poster
CC=
https://codereview.appspot.com/19870043

https://codereview.appspot.com/19870043/diff/1/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/19870043/diff/1/quickstart/utils.py#newcode35
quickstart/utils.py:35: # Switch to charmworld API 3 when the 500 error
is fixed.
On 2013/10/30 20:58:55, gary.poster wrote:
> We had a fix and a deployment, and
> http://manage.jujucharms.com/api/3/charm/precise/juju-gui works now
(yay!)

Cool! Done.

https://codereview.appspot.com/19870043/

Revision history for this message
Francesco Banconi (frankban) wrote :

Thanks for the review Gary!

https://codereview.appspot.com/19870043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'quickstart/app.py'
2--- quickstart/app.py 2013-10-30 10:16:36 +0000
3+++ quickstart/app.py 2013-10-31 08:49:45 +0000
4@@ -19,6 +19,7 @@
5 from __future__ import print_function
6 import functools
7 import json
8+import logging
9
10 import jujuclient
11
12@@ -28,6 +29,11 @@
13 )
14
15
16+# The default Juju GUI charm URL to use when it is not possible to retrieve it
17+# from the charmworld API, e.g. due to temporary connection/charmworld errors.
18+DEFAULT_CHARM_URL = 'cs:precise/juju-gui-78'
19+
20+
21 class ProgramExit(Exception):
22 """An error occurred while setting up the Juju environment.
23
24@@ -113,10 +119,12 @@
25
26 Raise a ProgramExit if the API server returns an error response.
27 """
28- # XXX 2013-10-17 frankban:
29- # Retrieve the URL of the last charm revision from
30- # manage.jujucharms.com.
31- charm_url = 'cs:precise/juju-gui-78'
32+ try:
33+ charm_url = utils.get_charm_url()
34+ except (IOError, ValueError) as err:
35+ msg = 'unable to retrieve the Juju GUI charm URL from the API: {}'
36+ logging.warn(msg.format(err))
37+ charm_url = DEFAULT_CHARM_URL
38 try:
39 env.deploy(service_name, charm_url, to=0)
40 env.expose(service_name)
41
42=== modified file 'quickstart/manage.py'
43--- quickstart/manage.py 2013-10-30 10:16:36 +0000
44+++ quickstart/manage.py 2013-10-31 08:49:45 +0000
45@@ -46,21 +46,26 @@
46 """Validate and process the bundle options.
47
48 Populate the options namespace with the following names:
49- - bundle_file: the full path to the bundle file;
50 - bundle_name: the name of the bundle;
51 - bundle_services: a list of service names included in the bundle;
52 - bundle_yaml: the YAML encoded contents of the bundle.
53
54 Exit with an error if the bundle options are not valid.
55 """
56- # XXX 2013-10-18 frankban:
57- # This function is supposed to also support bundle URLs.
58- bundle_file = os.path.abspath(os.path.expanduser(options.bundle))
59- # Load the bundle file.
60- try:
61- bundle_yaml = open(bundle_file).read()
62- except IOError as err:
63- return parser.error('unable to open bundle file: {}'.format(err))
64+ bundle = options.bundle
65+ if bundle.startswith('http://') or bundle.startswith('https://'):
66+ # Load the bundle URL.
67+ try:
68+ bundle_yaml = utils.urlread(bundle)
69+ except IOError as err:
70+ return parser.error('unable to open bundle URL: {}'.format(err))
71+ else:
72+ # Load the bundle file.
73+ bundle_file = os.path.abspath(os.path.expanduser(bundle))
74+ try:
75+ bundle_yaml = open(bundle_file).read()
76+ except IOError as err:
77+ return parser.error('unable to open bundle file: {}'.format(err))
78 # Validate the bundle.
79 try:
80 bundle_name, bundle_services = utils.parse_bundle(
81@@ -68,7 +73,6 @@
82 except ValueError as err:
83 return parser.error(str(err))
84 # Update the options namespace with the new values.
85- options.bundle_file = bundle_file
86 options.bundle_name = bundle_name
87 options.bundle_services = bundle_services
88 options.bundle_yaml = bundle_yaml
89@@ -140,11 +144,9 @@
90 env_help = '{} (%(default)s)'.format(env_help)
91 # Create and set up the arguments parser.
92 parser = argparse.ArgumentParser(description=quickstart.__doc__)
93- # XXX 2013-10-18 frankban:
94- # Make it possible to pass a URL as bundle argument.
95 parser.add_argument(
96 'bundle', default=None, nargs='?',
97- help='The path to the bundle file to deploy')
98+ help='The bundle URL or the path to the bundle file to deploy')
99 parser.add_argument(
100 '-e', '--environment', default=default_env_name, dest='env_name',
101 help=env_help)
102
103=== modified file 'quickstart/tests/helpers.py'
104--- quickstart/tests/helpers.py 2013-10-30 10:16:36 +0000
105+++ quickstart/tests/helpers.py 2013-10-31 08:49:45 +0000
106@@ -100,6 +100,23 @@
107 return env_file.name
108
109
110+class UrlReadTestsMixin(object):
111+ """Expose a method to mock the quickstart.utils.urlread helper function."""
112+
113+ def patch_urlread(self, contents=None, error=False):
114+ """Patch the quickstart.utils.urlread helper function.
115+
116+ If contents is not None, urlread() will return the provided contents.
117+ If error is set to True, an IOError will be simulated.
118+ """
119+ mock_urlread = mock.Mock()
120+ if contents is not None:
121+ mock_urlread.return_value = contents
122+ if error:
123+ mock_urlread.side_effect = IOError('bad wolf')
124+ return mock.patch('quickstart.utils.urlread', mock_urlread)
125+
126+
127 class ValueErrorTestsMixin(object):
128 """Set up some base methods for testing functions raising ValueErrors."""
129
130
131=== modified file 'quickstart/tests/test_app.py'
132--- quickstart/tests/test_app.py 2013-10-30 10:16:36 +0000
133+++ quickstart/tests/test_app.py 2013-10-31 08:49:45 +0000
134@@ -235,12 +235,37 @@
135
136 class TestDeployGui(ProgramExitTestsMixin, unittest.TestCase):
137
138+ charm_url = 'cs:precise/juju-gui-42'
139+
140+ def patch_get_charm_url(self, side_effect=None):
141+ """Patch the get_charm_url helper function."""
142+ if side_effect is None:
143+ side_effect = [self.charm_url]
144+ mock_get_charm_url = mock.Mock(side_effect=side_effect)
145+ return mock.patch('quickstart.utils.get_charm_url', mock_get_charm_url)
146+
147 def test_deployment(self):
148- # The function correctly deploys and exposes the service.
149- env = mock.Mock()
150- app.deploy_gui(env, 'my-gui')
151- env.assert_has_calls([
152- mock.call.deploy('my-gui', 'cs:precise/juju-gui-78', to=0),
153+ # The function correctly deploys and exposes the service, retrieving
154+ # the charm URL from the charmworld API.
155+ env = mock.Mock()
156+ with self.patch_get_charm_url():
157+ app.deploy_gui(env, 'my-gui')
158+ env.assert_has_calls([
159+ mock.call.deploy('my-gui', 'cs:precise/juju-gui-42', to=0),
160+ mock.call.expose('my-gui')
161+ ])
162+
163+ def test_deployment_default_charm_url(self):
164+ # The function correctly deploys and exposes the service, even if it is
165+ # not able to retrieve the charm URL from the charmworld API.
166+ env = mock.Mock()
167+ log = 'unable to retrieve the Juju GUI charm URL from the API: boo!'
168+ with self.patch_get_charm_url(side_effect=IOError('boo!')):
169+ # A warning is logged which notifies we are using the default URL.
170+ with helpers.assert_logs([log], level='warn'):
171+ app.deploy_gui(env, 'my-gui')
172+ env.assert_has_calls([
173+ mock.call.deploy('my-gui', app.DEFAULT_CHARM_URL, to=0),
174 mock.call.expose('my-gui')
175 ])
176
177@@ -249,20 +274,22 @@
178 env = mock.Mock()
179 env.deploy.side_effect = self.make_env_error('service already exists')
180 expected = 'bad API server response: service already exists'
181- with self.assert_program_exit(expected):
182- app.deploy_gui(env, 'another-gui')
183+ with self.patch_get_charm_url():
184+ with self.assert_program_exit(expected):
185+ app.deploy_gui(env, 'another-gui')
186 env.deploy.assert_called_once_with(
187- 'another-gui', 'cs:precise/juju-gui-78', to=0)
188+ 'another-gui', 'cs:precise/juju-gui-42', to=0)
189
190 def test_other_errors(self):
191 # Any other errors occurred during the process are not trapped.
192 error = ValueError('explode!')
193 env = mock.Mock()
194 env.expose.side_effect = error
195- with self.assertRaises(ValueError) as context_manager:
196- app.deploy_gui(env, 'juju-gui')
197+ with self.patch_get_charm_url():
198+ with self.assertRaises(ValueError) as context_manager:
199+ app.deploy_gui(env, 'juju-gui')
200 env.deploy.assert_called_once_with(
201- 'juju-gui', 'cs:precise/juju-gui-78', to=0)
202+ 'juju-gui', 'cs:precise/juju-gui-42', to=0)
203 env.expose.assert_called_once_with('juju-gui')
204 self.assertIs(error, context_manager.exception)
205
206
207=== modified file 'quickstart/tests/test_manage.py'
208--- quickstart/tests/test_manage.py 2013-10-30 10:16:36 +0000
209+++ quickstart/tests/test_manage.py 2013-10-31 08:49:45 +0000
210@@ -46,7 +46,9 @@
211 mock_exit.assert_called_once_with(0)
212
213
214-class TestValidateBundle(helpers.BundleFileTestsMixin, unittest.TestCase):
215+class TestValidateBundle(
216+ helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin,
217+ unittest.TestCase):
218
219 def setUp(self):
220 self.parser = mock.Mock()
221@@ -55,12 +57,24 @@
222 """Return a mock options object which includes the passed arguments."""
223 return mock.Mock(bundle=bundle, bundle_name=bundle_name)
224
225- def test_resulting_options(self):
226- # The options object is correctly set up.
227+ def test_resulting_options_from_file(self):
228+ # The options object is correctly set up when a bundle file is passed.
229 bundle_file = self.make_bundle_file()
230 options = self.make_options(bundle_file, bundle_name='bundle1')
231 manage._validate_bundle(options, self.parser)
232- self.assertEqual(bundle_file, options.bundle_file)
233+ self.assertEqual('bundle1', options.bundle_name)
234+ self.assertEqual(
235+ ['mysql', 'wordpress'], sorted(options.bundle_services))
236+ self.assertEqual(open(bundle_file).read(), options.bundle_yaml)
237+
238+ def test_resulting_options_from_url(self):
239+ # The options object is correctly set up when a bundle URL is passed.
240+ bundle_file = self.make_bundle_file()
241+ url = 'http://example.com/bundle.yaml'
242+ options = self.make_options(url, bundle_name='bundle1')
243+ with self.patch_urlread(contents=self.valid_bundle) as mock_urlread:
244+ manage._validate_bundle(options, self.parser)
245+ mock_urlread.assert_called_once_with(url)
246 self.assertEqual('bundle1', options.bundle_name)
247 self.assertEqual(
248 ['mysql', 'wordpress'], sorted(options.bundle_services))
249@@ -78,7 +92,7 @@
250 options = self.make_options(bundle=path, bundle_name='bundle2')
251 with mock.patch('os.environ', {'HOME': base_path}):
252 manage._validate_bundle(options, self.parser)
253- self.assertEqual(bundle_file, options.bundle_file)
254+ self.assertEqual(self.valid_bundle, options.bundle_yaml)
255
256 def test_bundle_file_not_found(self):
257 # A parser error is invoked if the bundle file is not found.
258@@ -90,8 +104,19 @@
259 )
260 self.parser.error.assert_called_once_with(expected)
261
262- def test_error_parsing_bundle_file(self):
263- # A parser error is invoked if an error occurs parsing the bundle file.
264+ def test_url_error(self):
265+ # A parser error is invoked if the bundle cannot be fetched from the
266+ # provided URL.
267+ url = 'http://example.com/bundle.yaml'
268+ options = self.make_options(url)
269+ with self.patch_urlread(error=True) as mock_urlread:
270+ manage._validate_bundle(options, self.parser)
271+ mock_urlread.assert_called_once_with(url)
272+ self.parser.error.assert_called_once_with(
273+ 'unable to open bundle URL: bad wolf')
274+
275+ def test_error_parsing_bundle_contents(self):
276+ # A parser error is invoked if an error occurs parsing the bundle YAML.
277 bundle_file = self.make_bundle_file()
278 options = self.make_options(bundle_file, bundle_name='no-such')
279 manage._validate_bundle(options, self.parser)
280
281=== modified file 'quickstart/tests/test_utils.py'
282--- quickstart/tests/test_utils.py 2013-10-30 10:16:36 +0000
283+++ quickstart/tests/test_utils.py 2013-10-31 08:49:45 +0000
284@@ -16,7 +16,11 @@
285
286 """Tests for the Juju Quickstart utility functions and classes."""
287
288+import httplib
289+import json
290+import socket
291 import unittest
292+import urllib2
293
294 import mock
295 import yaml
296@@ -71,6 +75,35 @@
297 utils.call('echo', 'we are the borg!')
298
299
300+class TestGetCharmUrl(helpers.UrlReadTestsMixin, unittest.TestCase):
301+
302+ def test_charm_url(self):
303+ # The Juju GUI charm URL is correctly returned.
304+ contents = json.dumps({'charm': {'url': 'cs:precise/juju-gui-42'}})
305+ with self.patch_urlread(contents=contents) as mock_urlread:
306+ charm_url = utils.get_charm_url()
307+ self.assertEqual('cs:precise/juju-gui-42', charm_url)
308+ mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
309+
310+ def test_io_error(self):
311+ # IOErrors are properly propagated.
312+ with self.patch_urlread(error=True) as mock_urlread:
313+ with self.assertRaises(IOError) as context_manager:
314+ utils.get_charm_url()
315+ mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
316+ self.assertEqual('bad wolf', str(context_manager.exception))
317+
318+ def test_value_error(self):
319+ # A ValueError is raised if the API response is not valid.
320+ contents = json.dumps({'charm': {}})
321+ with self.patch_urlread(contents=contents) as mock_urlread:
322+ with self.assertRaises(ValueError) as context_manager:
323+ utils.get_charm_url()
324+ mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
325+ self.assertEqual(
326+ 'unable to find the charm URL', str(context_manager.exception))
327+
328+
329 class TestGetDefaultEnvName(helpers.CallTestsMixin, unittest.TestCase):
330
331 def test_environment_variable(self):
332@@ -354,6 +387,46 @@
333 utils.unit_changes(self.unit, changeset))
334
335
336+class TestUrlread(unittest.TestCase):
337+
338+ def patch_urlopen(self, contents=None, error=None):
339+ """Patch the urllib2.urlopen function.
340+
341+ If contents is not None, the read() method of the returned mock object
342+ returns the given contents.
343+ If an error is provided, the call raises the error.
344+ """
345+ mock_urlopen = mock.Mock()
346+ if contents is not None:
347+ mock_urlopen().read.return_value = contents
348+ if error is not None:
349+ mock_urlopen.side_effect = error
350+ mock_urlopen.reset_mock()
351+ return mock.patch('urllib2.urlopen', mock_urlopen)
352+
353+ def test_contents(self):
354+ # The URL contents are correctly returned.
355+ with self.patch_urlopen(contents='URL contents') as mock_urlopen:
356+ contents = utils.urlread('http://example.com/path/')
357+ self.assertEqual('URL contents', contents)
358+ mock_urlopen.assert_called_once_with('http://example.com/path/')
359+
360+ def test_errors(self):
361+ # An IOError is raised if an error occurs connecting to the API.
362+ errors = {
363+ 'httplib HTTPException': httplib.HTTPException,
364+ 'socket error': socket.error,
365+ 'urllib2 URLError': urllib2.URLError,
366+ }
367+ for message, exception_class in errors.items():
368+ exception = exception_class(message)
369+ with self.patch_urlopen(error=exception) as mock_urlopen:
370+ with self.assertRaises(IOError) as context_manager:
371+ utils.urlread('http://example.com/path/')
372+ mock_urlopen.assert_called_once_with('http://example.com/path/')
373+ self.assertEqual(message, str(context_manager.exception))
374+
375+
376 class TestUtf8(unittest.TestCase):
377
378 def test_unicode(self):
379
380=== modified file 'quickstart/utils.py'
381--- quickstart/utils.py 2013-10-30 10:16:36 +0000
382+++ quickstart/utils.py 2013-10-31 08:49:45 +0000
383@@ -17,16 +17,22 @@
384 """Juju Quickstart utility functions and classes."""
385
386 import collections
387+import httplib
388+import json
389 import logging
390 import os
391 import pipes
392 import re
393+import socket
394 import subprocess
395+import urllib2
396
397 import yaml
398
399 # Compile the regular expression used to parse the "juju switch" output.
400 _juju_switch_expression = re.compile(r'Current environment: "([\w-]+)"\n')
401+# Define the URL containing information about the last Juju GUI charm version.
402+CHARMWORLD_API = 'http://manage.jujucharms.com/api/3/charm/precise/juju-gui'
403
404
405 def call(*args):
406@@ -52,6 +58,19 @@
407 return retcode, output, error
408
409
410+def get_charm_url():
411+ """Return the charm URL of the latest Juju GUI charm revision.
412+
413+ Raise an IOError if any problems occur connecting to the API endpoint.
414+ Raise a ValueError if the API returns invalid data.
415+ """
416+ charm_info = json.loads(urlread(CHARMWORLD_API))
417+ charm_url = charm_info.get('charm', {}).get('url')
418+ if charm_url is None:
419+ raise ValueError('unable to find the charm URL')
420+ return charm_url
421+
422+
423 def get_default_env_name():
424 """Return the current Juju environment name.
425
426@@ -212,6 +231,20 @@
427 return change
428
429
430+def urlread(url):
431+ """Open the given URL and return the page contents.
432+
433+ Raise an IOError if any problems occur.
434+ """
435+ try:
436+ response = urllib2.urlopen(url)
437+ except urllib2.URLError as err:
438+ raise IOError(err.reason)
439+ except (httplib.HTTPException, socket.error, urllib2.HTTPError) as err:
440+ raise IOError(str(err))
441+ return response.read()
442+
443+
444 def utf8(value):
445 """Return the utf8 encoded version of the given value.
446

Subscribers

People subscribed via source and target branches