Merge lp:~frankban/juju-quickstart/old-style-bundles-regression into lp:juju-quickstart
- old-style-bundles-regression
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 123 |
Proposed branch: | lp:~frankban/juju-quickstart/old-style-bundles-regression |
Merge into: | lp:juju-quickstart |
Diff against target: |
1630 lines (+868/-269) 15 files modified
quickstart/__init__.py (+1/-1) quickstart/app.py (+4/-2) quickstart/charmstore.py (+161/-0) quickstart/manage.py (+5/-5) quickstart/models/bundles.py (+136/-59) quickstart/models/references.py (+9/-48) quickstart/netutils.py (+9/-18) quickstart/settings.py (+2/-2) quickstart/tests/helpers.py (+5/-4) quickstart/tests/models/test_bundles.py (+166/-25) quickstart/tests/models/test_references.py (+0/-37) quickstart/tests/test_app.py (+31/-27) quickstart/tests/test_charmstore.py (+313/-0) quickstart/tests/test_netutils.py (+25/-40) quickstart/utils.py (+1/-1) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/old-style-bundles-regression |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+252353@code.launchpad.net |
Commit message
Description of the change
Fix the old-style bundle regression.
This branch fixes
https:/
In essence, legacy bundles are converted by the
ingestion process like the following:
- if a basked includes multiple bundles, the resulting
v4 bundle name is {basket-
each {bundle-name};
- if a basket only includes one bundle, the v4 bundle
is just {basket-name}.
Previously quickstart always assumed the former: this
branch adds a check for the latter before exiting with
an error.
This branch also introduces a charmstore module in
quickstart. All the interactions between quickstart
and the charm store are now collected in this module.
As part of this refactoring, quickstart is now able
to distinguish HTTP 404 errors and all the other
generic IOErrors that can be raised when connecting
to the network.
Also simplified the logging format and bootstrap logging
earlier in the application execution.
Tests: `make check`.
QA:
install bundles with quickstart:
`devenv/
Try the following bundles:
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
Those instead should return errors:
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
Francesco Banconi (frankban) wrote : | # |
Jeff Pihach (hatch) wrote : | # |
LTGM QA OK
Jeff Pihach (hatch) wrote : | # |
Thanks a lot for this fix. really cleans up a lot of the code!
Richard Harding (rharding) wrote : | # |
Thanks for this branch Francesco, I've mainly got one big question on
the legacy version of the call and some minor questions on just code
bits.
Will make sure to look asap in the morning. Let me know if anything here
is unclear.
https:/
File quickstart/
https:/
quickstart/
can this just use the netutils NotFoundError?
https:/
quickstart/
are these internal? Should they be _get prefixed or anything as internal
use methods?
https:/
quickstart/
path.lstrip('/')
can you confirm that the settings will allow overriding the api endpoint
via env var? Is that documented, maybe in the hacking doc?
https:/
quickstart/
Ok, so this is intentional to tell which kind of "NotFoundError" you've
got by if it's a netutils vs a charmstore.py one? Should we name them
different or just use the traceback?
https:/
quickstart/
charm reference, use the following:
here you mention a charm reference but this is particular to bundles.
Should this be bundle reference?
https:/
quickstart/
what about specifying the version? The main concern is that if it
doesn't exist that the failure is consistent or clear to the user.
https:/
quickstart/
do we ever get the legacy bundle though now? I thought we only got the
updated bundle and then turned it into legacy format by nesting it
inside another level?
https:/
quickstart/
path):
get_bundle_data vs _retrieve_
we can get the word 'parse' into the function name to help clarify that
one fetches while the other actually parses?
https:/
File quickstart/
https:/
quickstart/
charmstore.
I was expecting this to use the same 'get_bundle_data but with the
different reference using only the basket vs getting the orig.yaml file
using the get_legacy_
https:/
quickstart/mo...
Brad Crittenden (bac) wrote : | # |
LGTM. Thanks for the branch. Did not QA.
https:/
File quickstart/
https:/
quickstart/
the given reference cannot be
typo: the an entity
https:/
File quickstart/
https:/
quickstart/
Thank you for the super-clear regex construction.
- 130. By Francesco Banconi
-
Changes as per review.
Francesco Banconi (frankban) wrote : | # |
Please take a look.
https:/
File quickstart/
https:/
quickstart/
On 2015/03/10 01:18:32, rharding wrote:
> can this just use the netutils NotFoundError?
It can. But I didn't implement it lije that on purpose.
My idea is that having the charmstore raise a charmstore.
is part of the contract. Instead, exposing its internal dependency on
netutils.urlread is not. This is why we basically mask the underlying
exception.
With this implementation, we discourage people to write something like:
try:
charmstore.
except netutils.
...
The code above would establish a connection between the charmstore and
the netutils code, which is only an implementation detail. If, for
instance in the future we decide to replace the simplistic code in
netutils with something like requests or similar, the current API
doesn't have to change, i.e. the charmstore would still raise a
charmstore.
In essence, clients using the charmstore API doesn't have to know (or
import) netutils, and vice versa.
I considered the module namespace to be good enough to distinguish
between netutils.
That is the reasoning behind this implementation, and I agree it can
seem a repetition, but it's separation of concerns in my idea. Of course
feel free to disagree with the above.
https:/
quickstart/
On 2015/03/10 01:18:32, rharding wrote:
> are these internal? Should they be _get prefixed or anything as
internal use
> methods?
The idea is that this is public API. A client can use this function when
no higher level calls are available.
https:/
quickstart/
path.lstrip('/')
On 2015/03/10 01:18:32, rharding wrote:
> can you confirm that the settings will allow overriding the api
endpoint via env
> var? Is that documented, maybe in the hacking doc?
Overriding the charmstore API URL is not supported currently. Do we need
to support that? If so, I'll create a card.
https:/
quickstart/
On 2015/03/10 01:18:32, rharding wrote:
> Ok, so this is intentional to tell which kind of "NotFoundError"
you've got by
> if it's a netutils vs a charmstore.py one? Should we name them
different or just
> use the traceback?
Not sure if I understand the question about using the traceback. See
above for the goal of having two separate exception types.
https:/
quickstart/
charm reference, use the following:
On 2015/03/10 01:18:32, rharding wrote:
> here you mention a charm reference but this is particula...
Richard Harding (rharding) wrote : | # |
On 2015/03/10 10:20:07, frankban wrote:
> Please take a look.
https:/
> File quickstart/
https:/
> quickstart/
> On 2015/03/10 01:18:32, rharding wrote:
> > can this just use the netutils NotFoundError?
> It can. But I didn't implement it lije that on purpose.
> My idea is that having the charmstore raise a charmstore.
is part
> of the contract. Instead, exposing its internal dependency on
netutils.urlread
> is not. This is why we basically mask the underlying exception.
> With this implementation, we discourage people to write something
like:
> try:
> charmstore.get(...)
> except netutils.
> ...
> The code above would establish a connection between the charmstore and
the
> netutils code, which is only an implementation detail. If, for
instance in the
> future we decide to replace the simplistic code in netutils with
something like
> requests or similar, the current API doesn't have to change, i.e. the
charmstore
> would still raise a charmstore.
> In essence, clients using the charmstore API doesn't have to know (or
import)
> netutils, and vice versa.
> I considered the module namespace to be good enough to distinguish
between
> netutils.
> That is the reasoning behind this implementation, and I agree it can
seem a
> repetition, but it's separation of concerns in my idea. Of course feel
free to
> disagree with the above.
https:/
> quickstart/
> On 2015/03/10 01:18:32, rharding wrote:
> > are these internal? Should they be _get prefixed or anything as
internal use
> > methods?
> The idea is that this is public API. A client can use this function
when no
> higher level calls are available.
https:/
> quickstart/
path.lstrip('/')
> On 2015/03/10 01:18:32, rharding wrote:
> > can you confirm that the settings will allow overriding the api
endpoint via
> env
> > var? Is that documented, maybe in the hacking doc?
> Overriding the charmstore API URL is not supported currently. Do we
need to
> support that? If so, I'll create a card.
https:/
> quickstart/
> On 2015/03/10 01:18:32, rharding wrote:
> > Ok, so this is intentional to tell which kind of "NotFoundError"
you've got by
> > if it's a netutils vs a charmstore.py one? Should we name them
different or
> just
> > use the traceback?
> Not sure if I understand the question about using the traceback. See
above for
> the goal of having two separate exception types.
https:/
> quickstart/
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Fix the old-style bundle regression.
This branch fixes
https:/
In essence, legacy bundles are converted by the
ingestion process like the following:
- if a basked includes multiple bundles, the resulting
v4 bundle name is {basket-
each {bundle-name};
- if a basket only includes one bundle, the v4 bundle
is just {basket-name}.
Previously quickstart always assumed the former: this
branch adds a check for the latter before exiting with
an error.
This branch also introduces a charmstore module in
quickstart. All the interactions between quickstart
and the charm store are now collected in this module.
As part of this refactoring, quickstart is now able
to distinguish HTTP 404 errors and all the other
generic IOErrors that can be raised when connecting
to the network.
Also simplified the logging format and bootstrap logging
earlier in the application execution.
Tests: `make check`.
QA:
install bundles with quickstart:
`devenv/
Try the following bundles:
- devenv/
- devenv/
- devenv/
- devenv/
bundle:
- devenv/
Those instead should return errors:
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
bundle:
R=jeff.pihach, rharding, bac
CC=
https:/
Francesco Banconi (frankban) wrote : | # |
Thanks for the reviews!
Preview Diff
1 | === modified file 'quickstart/__init__.py' |
2 | --- quickstart/__init__.py 2015-02-27 09:25:33 +0000 |
3 | +++ quickstart/__init__.py 2015-03-10 10:19:09 +0000 |
4 | @@ -45,7 +45,7 @@ |
5 | Once Juju has been installed, the command can also be run as a juju plugin, |
6 | without the hyphen ("juju quickstart"). |
7 | """ |
8 | -VERSION = (2, 0, 0) |
9 | +VERSION = (2, 0, 1) |
10 | |
11 | |
12 | def get_version(): |
13 | |
14 | === modified file 'quickstart/app.py' |
15 | --- quickstart/app.py 2015-02-26 11:02:57 +0000 |
16 | +++ quickstart/app.py 2015-03-10 10:19:09 +0000 |
17 | @@ -29,6 +29,7 @@ |
18 | import jujuclient |
19 | |
20 | from quickstart import ( |
21 | + charmstore, |
22 | juju, |
23 | jujutools, |
24 | netutils, |
25 | @@ -440,8 +441,9 @@ |
26 | series = settings.JUJU_GUI_SUPPORTED_SERIES[-1] |
27 | try: |
28 | # Try to get the charm URL from the charm store API. |
29 | - charm_url = netutils.get_charm_url(series) |
30 | - except (IOError, ValueError) as err: |
31 | + charm_url = charmstore.resolve( |
32 | + settings.JUJU_GUI_CHARM_NAME, series=series) |
33 | + except (charmstore.NotFoundError, IOError, ValueError) as err: |
34 | # Fall back to the default URL for the current series. |
35 | msg = 'unable to retrieve the {} charm URL from the API: {}' |
36 | logging.warn(msg.format(service_name, err)) |
37 | |
38 | === added file 'quickstart/charmstore.py' |
39 | --- quickstart/charmstore.py 1970-01-01 00:00:00 +0000 |
40 | +++ quickstart/charmstore.py 2015-03-10 10:19:09 +0000 |
41 | @@ -0,0 +1,161 @@ |
42 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
43 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
44 | +# Copyright (C) 2015 Canonical Ltd. |
45 | +# |
46 | +# This program is free software: you can redistribute it and/or modify it under |
47 | +# the terms of the GNU Affero General Public License version 3, as published by |
48 | +# the Free Software Foundation. |
49 | +# |
50 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
51 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
52 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
53 | +# Affero General Public License for more details. |
54 | +# |
55 | +# You should have received a copy of the GNU Affero General Public License |
56 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
57 | + |
58 | +"""Juju Quickstart utilities for communicating with the Juju charm store.""" |
59 | + |
60 | +from __future__ import unicode_literals |
61 | + |
62 | +import json |
63 | + |
64 | +from quickstart import ( |
65 | + netutils, |
66 | + serializers, |
67 | + settings, |
68 | +) |
69 | + |
70 | + |
71 | +class NotFoundError(Exception): |
72 | + """Represent a not found HTTP error while communicating with the store.""" |
73 | + |
74 | + |
75 | +def get(path): |
76 | + """Send a GET request to the charm store at the given path. |
77 | + |
78 | + The path must not include the charm store API version identifier. In |
79 | + essence, the path must exclude the "/v4/" fragment. |
80 | + |
81 | + Return the string content returned by the charm store. |
82 | + Raise a NotFoundError if a 404 not found response is returned. |
83 | + Raise an IOError if any other problems occur while communicating with the |
84 | + charm store. |
85 | + """ |
86 | + url = settings.CHARMSTORE_API + path.lstrip('/') |
87 | + try: |
88 | + return netutils.urlread(url) |
89 | + except netutils.NotFoundError as err: |
90 | + msg = b'charm store resource not found at {}: {}'.format( |
91 | + url.encode('utf-8'), err) |
92 | + raise NotFoundError(msg) |
93 | + except IOError as err: |
94 | + msg = b'cannot communicate with the charm store at {}: {}'.format( |
95 | + url.encode('utf-8'), err) |
96 | + raise IOError(msg) |
97 | + |
98 | + |
99 | +def get_reference(reference, path): |
100 | + """Retrieve the charm store contents for the given reference and path. |
101 | + |
102 | + The reference argument identifies a charm or bundle entity and must be an |
103 | + instance of "quickstart.models.references.Reference". |
104 | + |
105 | + For instance, to retrieve the hash of a charm reference, use the following: |
106 | + |
107 | + hash = get_reference(reference, '/meta/hash') |
108 | + |
109 | + Raise a NotFoundError if an entity with the given reference cannot be |
110 | + found in the charm store. |
111 | + Raise an IOError if any other problems occur while communicating with the |
112 | + charm store. |
113 | + """ |
114 | + if not path.startswith('/'): |
115 | + path = '/' + path |
116 | + return get(reference.path() + path) |
117 | + |
118 | + |
119 | +def resolve(name, series=None): |
120 | + """Return the fully qualified id of the entity with the given name. |
121 | + |
122 | + If the optional series is provided, resolve the entity of the given series. |
123 | + |
124 | + Raise a NotFoundError if an entity with the given name and optional series |
125 | + cannot be found in the charm store. |
126 | + Raise an IOError if any other problems occur while communicating with the |
127 | + charm store. |
128 | + Raise a ValueError if the API returns invalid data. |
129 | + """ |
130 | + series_part = '' if series is None else '{}/'.format(series) |
131 | + content = get(series_part + name + '/meta/id') |
132 | + data = json.loads(content) |
133 | + entity_id = data.get('Id') |
134 | + if entity_id is None: |
135 | + msg = 'unable to resolve entity id {}{}'.format(series_part, name) |
136 | + raise ValueError(msg.encode('utf-8')) |
137 | + return entity_id |
138 | + |
139 | + |
140 | +def get_bundle_data(reference): |
141 | + """Retrieve and return the bundle data for the given bundle reference. |
142 | + |
143 | + The bundle data is returned as a YAML decoded value. |
144 | + The reference argument identifies a bundle entity and must be an instance |
145 | + of "quickstart.models.references.Reference". |
146 | + |
147 | + Raise a ValueError if the returned content is not a valid YAML, or if the |
148 | + given reference does not represent a bundle. |
149 | + Raise a NotFoundError if a bundle with the given reference cannot be found |
150 | + in the charm store. |
151 | + Raise an IOError if any other problems occur while communicating with the |
152 | + charm store. |
153 | + """ |
154 | + return _retrieve_and_parse_bundle_data(reference, '/archive/bundle.yaml') |
155 | + |
156 | + |
157 | +def get_legacy_bundle_data(reference): |
158 | + """Retrieve and return the bundle legacy data for the given reference. |
159 | + |
160 | + The bundle data is returned as a YAML decoded value and represents the |
161 | + legacy bundle with a top level bundle name node. |
162 | + The reference argument identifies a bundle entity and must be an instance |
163 | + of "quickstart.models.references.Reference". |
164 | + |
165 | + Raise a ValueError if the returned content is not a valid YAML, or if the |
166 | + given reference does not represent a bundle. |
167 | + Raise a NotFoundError if a bundle with the given reference cannot be found |
168 | + in the charm store. |
169 | + Raise an IOError if any other problems occur while communicating with the |
170 | + charm store. |
171 | + """ |
172 | + return _retrieve_and_parse_bundle_data( |
173 | + reference, '/archive/bundles.yaml.orig') |
174 | + |
175 | + |
176 | +def _retrieve_and_parse_bundle_data(reference, path): |
177 | + """Retrieve and parse the bundle YAML for the given reference and path. |
178 | + |
179 | + Raise a ValueError if the returned content is not a valid YAML, or if the |
180 | + given reference does not represent a bundle. |
181 | + Raise a NotFoundError if a bundle with the given reference cannot be found |
182 | + in the charm store. |
183 | + Raise an IOError if any other problems occur while communicating with the |
184 | + charm store. |
185 | + """ |
186 | + if not reference.is_bundle(): |
187 | + raise ValueError( |
188 | + b'expected a bundle, provided charm {}'.format(reference)) |
189 | + content = get_reference(reference, path) |
190 | + return load_bundle_yaml(content) |
191 | + |
192 | + |
193 | +def load_bundle_yaml(content): |
194 | + """Deserialize the given YAML encoded bundle content. |
195 | + |
196 | + Raise a ValueError if the content is not valid. |
197 | + """ |
198 | + try: |
199 | + return serializers.yaml_load(content) |
200 | + except Exception as err: |
201 | + msg = b'unable to parse the bundle content: {}'.format(err) |
202 | + raise ValueError(msg) |
203 | |
204 | === modified file 'quickstart/manage.py' |
205 | --- quickstart/manage.py 2015-02-26 12:42:34 +0000 |
206 | +++ quickstart/manage.py 2015-03-10 10:19:09 +0000 |
207 | @@ -294,7 +294,7 @@ |
208 | level=level, |
209 | format=( |
210 | '%(asctime)s %(levelname)s ' |
211 | - '%(module)s@%(funcName)s:%(lineno)d ' |
212 | + '%(module)s:%(lineno)d ' |
213 | '%(message)s' |
214 | ), |
215 | datefmt='%H:%M:%S', |
216 | @@ -459,9 +459,11 @@ |
217 | # Parse the provided arguments. |
218 | options = parser.parse_args() |
219 | |
220 | + # Set up logging. |
221 | + _configure_logging(logging.DEBUG if options.debug else logging.INFO) |
222 | + |
223 | + # Validate and add in the platform for convenience. |
224 | _validate_platform(parser, platform) |
225 | - |
226 | - # Add in the platform for convenience. |
227 | options.platform = platform |
228 | |
229 | # Convert the provided string arguments to unicode. |
230 | @@ -472,8 +474,6 @@ |
231 | _validate_bundle(options, parser) |
232 | if options.charm_url is not None: |
233 | _validate_charm_url(options, parser) |
234 | - # Set up logging. |
235 | - _configure_logging(logging.DEBUG if options.debug else logging.INFO) |
236 | return options |
237 | |
238 | |
239 | |
240 | === modified file 'quickstart/models/bundles.py' |
241 | --- quickstart/models/bundles.py 2015-02-26 19:46:48 +0000 |
242 | +++ quickstart/models/bundles.py 2015-03-10 10:19:09 +0000 |
243 | @@ -44,8 +44,10 @@ |
244 | import collections |
245 | import logging |
246 | import os |
247 | +import re |
248 | |
249 | from quickstart import ( |
250 | + charmstore, |
251 | netutils, |
252 | serializers, |
253 | settings, |
254 | @@ -140,11 +142,7 @@ |
255 | """ |
256 | if source.startswith('bundle:'): |
257 | # The source refers to an old style bundle URL. |
258 | - reference = references.Reference.from_charmworld_url(source) |
259 | - logging.warn( |
260 | - 'this bundle URL is deprecated: please use the new format: ' |
261 | - '{}'.format(reference.jujucharms_id())) |
262 | - return _bundle_from_reference(reference) |
263 | + return _bundle_from_charmworld_url(source) |
264 | |
265 | has_extension = source.endswith('.yaml') or source.endswith('.json') |
266 | is_remote = source.startswith('http://') or source.startswith('https://') |
267 | @@ -163,30 +161,84 @@ |
268 | # No other options are available. |
269 | raise |
270 | |
271 | - if not reference.is_bundle(): |
272 | - raise ValueError( |
273 | - b'expected a bundle, provided charm {}'.format(reference)) |
274 | - |
275 | # The source refers to a bundle URL in jujucharms.com. |
276 | - return _bundle_from_reference(reference) |
277 | - |
278 | - |
279 | -def _bundle_from_reference(reference): |
280 | - """Retrieve bundle YAML contents from its reference in the charm store. |
281 | - |
282 | - The path of an entity in the charm store is the fully qualified URL without |
283 | - the schema. The schema is implicitly set to "cs" (charm store entity), e.g. |
284 | - "vivid/django" or "~who/trusty/mediawiki-42". |
285 | - |
286 | - Return a Bundle instance which includes the retrieved data and the given |
287 | - reference. |
288 | + try: |
289 | + data = charmstore.get_bundle_data(reference) |
290 | + except charmstore.NotFoundError as err: |
291 | + raise IOError(bytes(err)) |
292 | + validate(data) |
293 | + return Bundle(data, reference=reference) |
294 | + |
295 | + |
296 | +# Compile the regular expression used to parse charmworld bundle URLs. |
297 | +_charmworld_url_expression = re.compile(r""" |
298 | + ^ # Beginning of the line. |
299 | + (?:bundle:) # Bundle schema. |
300 | + (?:~({user_pattern})/)? # Optional user name. |
301 | + ({name_pattern})/ # Basket name. |
302 | + (?:(\d+)/)? # Optional bundle revision number. |
303 | + ({name_pattern}) # Bundle name. |
304 | + /? # Optional trailing slash. |
305 | + $ # End of the line. |
306 | +""".format( |
307 | + name_pattern=references.NAME_PATTERN, |
308 | + user_pattern=references.USER_PATTERN, |
309 | +), re.VERBOSE) |
310 | + |
311 | + |
312 | +def _bundle_from_charmworld_url(url): |
313 | + """Retrieve bundle YAML contents from the given charmworld URL. |
314 | + |
315 | + These kind of "bundle:basket/name" URLs were used before the release |
316 | + of the new charm store (API version 4). Possible examples are |
317 | + "bundle:mediawiki/single" or "bundle:~who/wordpress/42/scalable". |
318 | + Note that charmworld URLs always represent a bundle. |
319 | + |
320 | + Return a Bundle instance which includes the retrieved data and the bundle |
321 | + reference corresponding to the given charmworld URL. |
322 | + Raise a ValueError if the provided URL is not valid, or if the bundle |
323 | + content is not valid. |
324 | Raise a IOError if a problem is encountered while fetching the YAML |
325 | content from the charm store. |
326 | - Raise a ValueError if the bundle content is not valid. |
327 | """ |
328 | - url = settings.CHARMSTORE_API + reference.path() + '/archive/bundle.yaml' |
329 | - content = _retrieve_from_url(url) |
330 | - data = parse_yaml(content) |
331 | + match = _charmworld_url_expression.match(url) |
332 | + if match is None: |
333 | + msg = 'invalid bundle URL: {}'.format(url) |
334 | + raise ValueError(msg.encode('utf-8')) |
335 | + user, basket, revision, name = match.groups() |
336 | + # The legacy bundle ingestion works like the following: |
337 | + # - if a basket includes multiple bundles, the resulting v4 bundle name is |
338 | + # "{basket-name}-{bundle-name}". In this case, a separate bundle is |
339 | + # created for each top level name found in the YAML; |
340 | + # - if a basket only includes one bundle, the v4 bundle just uses the |
341 | + # basket name, because no disambiguation is required. |
342 | + # For this reason, we cannot infer the new bundle identifier from a |
343 | + # charmworld URL: we instead need to try to fetch both references. |
344 | + fullname = '{}-{}'.format(basket, name) |
345 | + reference = references.Reference('cs', user, 'bundle', fullname, revision) |
346 | + try: |
347 | + data = charmstore.get_bundle_data(reference) |
348 | + except charmstore.NotFoundError: |
349 | + # Also try the case in which a single bundle was included in the |
350 | + # legacy YAML. In this case, validating that the name was included |
351 | + # in the original YAML is also required. |
352 | + reference = references.Reference( |
353 | + 'cs', user, 'bundle', basket, revision) |
354 | + try: |
355 | + data = charmstore.get_legacy_bundle_data(reference) |
356 | + except charmstore.NotFoundError as err: |
357 | + raise IOError(bytes(err)) |
358 | + data = _flatten_data(data, name) |
359 | + |
360 | + validate(data) |
361 | + # XXX frankban 2015-02-26: remove this when switching to the new bundle |
362 | + # format. Note that this is monkey patched on purpose: we don't want the |
363 | + # legacy bundle id to be part of the Reference class contract, and we don't |
364 | + # want to keep track of obsolete concepts such as "basket" there. |
365 | + reference.charmworld_id = url[len('bundle:'):] |
366 | + logging.warn( |
367 | + 'this bundle URL is deprecated: please use the new format: ' |
368 | + '{}'.format(reference.jujucharms_id())) |
369 | return Bundle(data, reference=reference) |
370 | |
371 | |
372 | @@ -198,7 +250,7 @@ |
373 | """ |
374 | try: |
375 | return netutils.urlread(url) |
376 | - except IOError as err: |
377 | + except (netutils.NotFoundError, IOError) as err: |
378 | msg = b'cannot retrieve bundle from remote URL {}: {}'.format( |
379 | url.encode('utf-8'), err) |
380 | raise IOError(msg) |
381 | @@ -233,12 +285,28 @@ |
382 | - the YAML contents are not properly structured; |
383 | - the bundle does not include services. |
384 | """ |
385 | - data = _open_yaml(content) |
386 | + data = charmstore.load_bundle_yaml(content) |
387 | # Validate the bundle data. |
388 | validate(data) |
389 | return data |
390 | |
391 | |
392 | +def is_legacy_bundle(data): |
393 | + """Report whether the given bundle data represents a legacy bundle. |
394 | + |
395 | + Assume the given data is a dictionary like object. |
396 | + """ |
397 | + services = data.get('services') |
398 | + # The internal structure of a bundle in the API version 4 does not include |
399 | + # a wrapping namespace with the bundle name. That's why the check below, |
400 | + # despite its ugliness, is quite effective. |
401 | + if (not services): |
402 | + return True |
403 | + if isinstance(services, collections.Mapping) and ('services' in services): |
404 | + return True |
405 | + return False |
406 | + |
407 | + |
408 | def _parse_and_flatten_yaml(content, name): |
409 | """Parse and validate the given bundle content. |
410 | |
411 | @@ -255,15 +323,29 @@ |
412 | than one bundle; |
413 | - the bundle does not include services. |
414 | """ |
415 | - data = _open_yaml(content) |
416 | - services = data.get('services') |
417 | - # The internal structure of a bundle in the API version 4 does not include |
418 | - # a wrapping namespace with the bundle name. That's why the check below, |
419 | - # despite its ugliness, is quite effective. |
420 | - if services and 'services' not in services: |
421 | - # This is an API version 4 bundle. |
422 | - validate(data) |
423 | - return data |
424 | + data = charmstore.load_bundle_yaml(content) |
425 | + _ensure_is_dict(data) |
426 | + if is_legacy_bundle(data): |
427 | + data = _flatten_data(data, name) |
428 | + validate(data) |
429 | + return data |
430 | + |
431 | + |
432 | +def _flatten_data(data, name): |
433 | + """Retrieve the bundle content from data for a specific bundle name. |
434 | + |
435 | + The returned YAML decoded data represents a new style bundle |
436 | + (API version 4). |
437 | + |
438 | + Raise a ValueError if: |
439 | + - the YAML data is not properly structured; |
440 | + - the YAML data does not include any bundles; |
441 | + - the bundle name is specified but not included in the bundle file; |
442 | + - the bundle name is not specified and the bundle file includes more |
443 | + than one bundle; |
444 | + - the bundle does not include services. |
445 | + """ |
446 | + _ensure_is_dict(data) |
447 | num_bundles = len(data) |
448 | if not num_bundles: |
449 | raise ValueError(b'no bundles found in the provided list of bundles') |
450 | @@ -272,30 +354,14 @@ |
451 | if num_bundles > 1: |
452 | msg = 'multiple bundles found ({}) but no bundle name specified' |
453 | raise ValueError(msg.format(names).encode('utf-8')) |
454 | - data = data.values()[0] |
455 | - else: |
456 | - data = data.get(name) |
457 | - if data is None: |
458 | + return data.values()[0] |
459 | + data = data.get(name) |
460 | + if data is None: |
461 | + if num_bundles == 1: |
462 | + msg = 'bundle {} not found, did you mean {}?' |
463 | + else: |
464 | msg = 'bundle {} not found in the provided list of bundles ({})' |
465 | - raise ValueError(msg.format(name, names).encode('utf-8')) |
466 | - validate(data) |
467 | - return data |
468 | - |
469 | - |
470 | -def _open_yaml(content): |
471 | - """Deserialize the given content, that must be a YAML encoded dictionary. |
472 | - |
473 | - Raise a ValueError if the content is not valid. |
474 | - """ |
475 | - try: |
476 | - data = serializers.yaml_load(content) |
477 | - except Exception as err: |
478 | - msg = b'unable to parse the bundle content: {}'.format(err) |
479 | - raise ValueError(msg) |
480 | - # Ensure the bundle content is well formed. |
481 | - if not isinstance(data, collections.Mapping): |
482 | - msg = 'invalid YAML content: {}'.format(data) |
483 | - raise ValueError(msg.encode('utf-8')) |
484 | + raise ValueError(msg.format(name, names).encode('utf-8')) |
485 | return data |
486 | |
487 | |
488 | @@ -312,6 +378,7 @@ |
489 | - the YAML contents are not properly structured; |
490 | - the bundle does not include services. |
491 | """ |
492 | + _ensure_is_dict(data) |
493 | # Retrieve the bundle services. |
494 | try: |
495 | services = data['services'].keys() |
496 | @@ -328,3 +395,13 @@ |
497 | b'the provided bundle contains an instance of juju-gui. Juju ' |
498 | b'Quickstart will install the latest version of the Juju GUI ' |
499 | b'automatically; please remove juju-gui from the bundle') |
500 | + |
501 | + |
502 | +def _ensure_is_dict(data): |
503 | + """Ensure that the given bundle data is a dictionary like object. |
504 | + |
505 | + Raise a ValueError otherwise. |
506 | + """ |
507 | + if not isinstance(data, collections.Mapping): |
508 | + msg = 'invalid YAML content: {}'.format(data) |
509 | + raise ValueError(msg.encode('utf-8')) |
510 | |
511 | === modified file 'quickstart/models/references.py' |
512 | --- quickstart/models/references.py 2015-02-27 09:25:33 +0000 |
513 | +++ quickstart/models/references.py 2015-03-10 10:19:09 +0000 |
514 | @@ -25,29 +25,15 @@ |
515 | |
516 | # The following regular expressions are the same used in juju-core: see |
517 | # http://bazaar.launchpad.net/~go-bot/juju-core/trunk/view/head:/charm/url.go. |
518 | -_USER_PATTERN = r'[a-z0-9][a-zA-Z0-9+.-]+' |
519 | -_SERIES_PATTERN = r'[a-z]+(?:[a-z-]+[a-z])?' |
520 | -_NAME_PATTERN = r'[a-z][a-z0-9]*(?:-[a-z0-9]*[a-z][a-z0-9]*)*' |
521 | +USER_PATTERN = r'[a-z0-9][a-zA-Z0-9+.-]+' |
522 | +SERIES_PATTERN = r'[a-z]+(?:[a-z-]+[a-z])?' |
523 | +NAME_PATTERN = r'[a-z][a-z0-9]*(?:-[a-z0-9]*[a-z][a-z0-9]*)*' |
524 | |
525 | # Define the callables used to check if entity reference components are valid. |
526 | -_valid_user = re.compile(r'^{}$'.format(_USER_PATTERN)).match |
527 | -_valid_series = re.compile(r'^{}$'.format(_SERIES_PATTERN)).match |
528 | -_valid_name = re.compile(r'^{}$'.format(_NAME_PATTERN)).match |
529 | +_valid_user = re.compile(r'^{}$'.format(USER_PATTERN)).match |
530 | +_valid_series = re.compile(r'^{}$'.format(SERIES_PATTERN)).match |
531 | +_valid_name = re.compile(r'^{}$'.format(NAME_PATTERN)).match |
532 | |
533 | -# Compile the regular expression used to parse charmworld bundle URLs. |
534 | -_charmworld_url_expression = re.compile(r""" |
535 | - ^ # Beginning of the line. |
536 | - (?:bundle:) # Bundle schema. |
537 | - (?:~({user_pattern})/)? # Optional user name. |
538 | - ({name_pattern})/ # Basket name. |
539 | - (?:(\d+)/)? # Optional bundle revision number. |
540 | - ({name_pattern}) # Bundle name. |
541 | - /? # Optional trailing slash. |
542 | - $ # End of the line. |
543 | -""".format( |
544 | - name_pattern=_NAME_PATTERN, |
545 | - user_pattern=_USER_PATTERN, |
546 | -), re.VERBOSE) |
547 | # Compile the regular expression used to parse new jujucharms entity URLs. |
548 | _jujucharms_url_expression = re.compile(r""" |
549 | ^ # Beginning of the line. |
550 | @@ -64,9 +50,9 @@ |
551 | $ # End of the line. |
552 | """.format( |
553 | jujucharms=settings.JUJUCHARMS_URL, |
554 | - name_pattern=_NAME_PATTERN, |
555 | - series_pattern=_SERIES_PATTERN, |
556 | - user_pattern=_USER_PATTERN, |
557 | + name_pattern=NAME_PATTERN, |
558 | + series_pattern=SERIES_PATTERN, |
559 | + user_pattern=USER_PATTERN, |
560 | ), re.VERBOSE) |
561 | |
562 | |
563 | @@ -101,31 +87,6 @@ |
564 | return cls(*_parse_fully_qualified_url(url)) |
565 | |
566 | @classmethod |
567 | - def from_charmworld_url(cls, url): |
568 | - """Create and return a Reference from the given charmworld URL. |
569 | - |
570 | - These kind of "bundle:basket/name" URLs were used before the release |
571 | - of the new charm store (API version 4). Possible examples are |
572 | - "bundle:mediawiki/single" or "bundle:~who/wordpress/42/scalable". |
573 | - Note that charmworld URLs always represent a bundle. |
574 | - |
575 | - Raise a ValueError if the provided URL is not valid. |
576 | - """ |
577 | - match = _charmworld_url_expression.match(url) |
578 | - if match is None: |
579 | - msg = 'invalid bundle URL: {}'.format(url) |
580 | - raise ValueError(msg.encode('utf-8')) |
581 | - user, basket, revision, name = match.groups() |
582 | - name = '{}-{}'.format(basket, name) |
583 | - self = cls('cs', user, 'bundle', name, revision) |
584 | - # XXX frankban 2015-02-26: remove this when switching to the new bundle |
585 | - # format. Note that this is monkey patched on purpose: we don't want |
586 | - # the legacy bundle id to be part of this class contract, and we don't |
587 | - # want to keep track of obsolete concepts such as "basket" here. |
588 | - self.charmworld_id = url[len('bundle:'):] |
589 | - return self |
590 | - |
591 | - @classmethod |
592 | def from_jujucharms_url(cls, url): |
593 | """Create and return a Reference from the given jujucharms.com URL. |
594 | |
595 | |
596 | === modified file 'quickstart/netutils.py' |
597 | --- quickstart/netutils.py 2015-02-25 15:28:56 +0000 |
598 | +++ quickstart/netutils.py 2015-03-10 10:19:09 +0000 |
599 | @@ -18,14 +18,11 @@ |
600 | |
601 | from __future__ import unicode_literals |
602 | |
603 | -import json |
604 | import httplib |
605 | import logging |
606 | import socket |
607 | import urllib2 |
608 | |
609 | -from quickstart import settings |
610 | - |
611 | |
612 | def check_resolvable(hostname): |
613 | """Check that the hostname can be resolved to a numeric IP address. |
614 | @@ -63,31 +60,25 @@ |
615 | return None |
616 | |
617 | |
618 | -def get_charm_url(series): |
619 | - """Return the charm URL of the latest Juju GUI charm revision. |
620 | - |
621 | - Raise an IOError if any problems occur connecting to the API endpoint. |
622 | - Raise a ValueError if the API returns invalid data. |
623 | - """ |
624 | - url = '{}{}/{}/meta/id'.format( |
625 | - settings.CHARMSTORE_API, series, settings.JUJU_GUI_CHARM_NAME) |
626 | - data = json.loads(urlread(url)) |
627 | - charm_url = data.get('Id') |
628 | - if charm_url is None: |
629 | - raise ValueError(b'unable to find the charm URL') |
630 | - return charm_url |
631 | +class NotFoundError(Exception): |
632 | + """Represent a 404 not found HTTP error.""" |
633 | |
634 | |
635 | def urlread(url): |
636 | """Open the given URL and return the page contents. |
637 | |
638 | - Raise an IOError if any problems occur. |
639 | + Raise a NotFoundError if the request returns a 404 not found response. |
640 | + Raise an IOError if any other problems occur. |
641 | """ |
642 | + logging.debug('sending HTTP GET request to {}'.format(url)) |
643 | try: |
644 | response = urllib2.urlopen(url) |
645 | + except urllib2.HTTPError as err: |
646 | + exception = NotFoundError if err.code == 404 else IOError |
647 | + raise exception(bytes(err)) |
648 | except urllib2.URLError as err: |
649 | raise IOError(err.reason) |
650 | - except (httplib.HTTPException, socket.error, urllib2.HTTPError) as err: |
651 | + except (httplib.HTTPException, socket.error) as err: |
652 | raise IOError(bytes(err)) |
653 | contents = response.read() |
654 | content_type = response.headers['content-type'] |
655 | |
656 | === modified file 'quickstart/settings.py' |
657 | --- quickstart/settings.py 2015-02-25 19:26:02 +0000 |
658 | +++ quickstart/settings.py 2015-03-10 10:19:09 +0000 |
659 | @@ -37,8 +37,8 @@ |
660 | # temporary connection/charm store errors. |
661 | # Keep this list sorted by release date (older first). |
662 | DEFAULT_CHARM_URLS = collections.OrderedDict(( |
663 | - ('precise', 'cs:precise/juju-gui-106'), |
664 | - ('trusty', 'cs:trusty/juju-gui-18'), |
665 | + ('precise', 'cs:precise/juju-gui-108'), |
666 | + ('trusty', 'cs:trusty/juju-gui-21'), |
667 | )) |
668 | |
669 | # The quickstart app short description. |
670 | |
671 | === modified file 'quickstart/tests/helpers.py' |
672 | --- quickstart/tests/helpers.py 2015-02-26 19:46:48 +0000 |
673 | +++ quickstart/tests/helpers.py 2015-03-10 10:19:09 +0000 |
674 | @@ -299,17 +299,18 @@ |
675 | class UrlReadTestsMixin(object): |
676 | """Helpers to mock the quickstart.netutils.urlread helper function.""" |
677 | |
678 | - def patch_urlread(self, contents=None, error=False): |
679 | + def patch_urlread(self, contents=None, error=None): |
680 | """Patch the quickstart.netutils.urlread helper function. |
681 | |
682 | If contents is not None, urlread() will return the provided contents. |
683 | - If error is set to True, an IOError will be simulated. |
684 | + Otherwise, if error is not None, the given exception or list of side |
685 | + effects will be simulated. |
686 | """ |
687 | mock_urlread = mock.Mock() |
688 | if contents is not None: |
689 | mock_urlread.return_value = contents |
690 | - if error: |
691 | - mock_urlread.side_effect = IOError('bad wolf') |
692 | + elif error is not None: |
693 | + mock_urlread.side_effect = error |
694 | return mock.patch('quickstart.netutils.urlread', mock_urlread) |
695 | |
696 | |
697 | |
698 | === modified file 'quickstart/tests/models/test_bundles.py' |
699 | --- quickstart/tests/models/test_bundles.py 2015-02-26 19:46:48 +0000 |
700 | +++ quickstart/tests/models/test_bundles.py 2015-03-10 10:19:09 +0000 |
701 | @@ -21,9 +21,13 @@ |
702 | import json |
703 | import unittest |
704 | |
705 | +import mock |
706 | import yaml |
707 | |
708 | -from quickstart import settings |
709 | +from quickstart import ( |
710 | + netutils, |
711 | + settings, |
712 | +) |
713 | from quickstart.models import ( |
714 | bundles, |
715 | references, |
716 | @@ -86,6 +90,10 @@ |
717 | bundle = bundles.from_source('bundle:mediawiki/single') |
718 | self.assertEqual(self.bundle_data, bundle.data) |
719 | self.assertEqual('cs:bundle/mediawiki-single', bundle.reference.id()) |
720 | + # The charmworld id is properly set when passing charmworld URLs. |
721 | + # XXX frankban 2015-03-09: remove this check once we get rid of the |
722 | + # charmworld id concept. |
723 | + self.assertEqual('mediawiki/single', bundle.reference.charmworld_id) |
724 | mock_urlread.assert_called_once_with( |
725 | settings.CHARMSTORE_API + |
726 | 'bundle/mediawiki-single/archive/bundle.yaml') |
727 | @@ -98,10 +106,45 @@ |
728 | self.assertEqual(self.bundle_data, bundle.data) |
729 | self.assertEqual( |
730 | 'cs:~who/bundle/mediawiki-single-42', bundle.reference.id()) |
731 | + # The charmworld id is properly set when passing charmworld URLs. |
732 | + # XXX frankban 2015-03-09: remove this check once we get rid of the |
733 | + # charmworld id concept. |
734 | + self.assertEqual( |
735 | + '~who/mediawiki/42/single', bundle.reference.charmworld_id) |
736 | mock_urlread.assert_called_once_with( |
737 | settings.CHARMSTORE_API + |
738 | '~who/bundle/mediawiki-single-42/archive/bundle.yaml') |
739 | |
740 | + def test_charmworld_bundle_from_legacy(self): |
741 | + # A bundle instance is properly returned from a charmworld id source |
742 | + # by looking at the legacy YAML stored in the charm store. |
743 | + side_effect = [ |
744 | + # A first call urlread returns a not found error. |
745 | + netutils.NotFoundError('boo!'), |
746 | + # A second call succeeds. |
747 | + self.legacy_bundle_content, |
748 | + ] |
749 | + with self.patch_urlread(error=side_effect) as mock_urlread: |
750 | + bundle = bundles.from_source('bundle:mediawiki/bundle1') |
751 | + self.assertEqual(self.bundle_data, bundle.data) |
752 | + self.assertEqual('cs:bundle/mediawiki', bundle.reference.id()) |
753 | + # The charmworld id is properly set when passing charmworld URLs. |
754 | + # XXX frankban 2015-03-09: remove this check once we get rid of the |
755 | + # charmworld id concept. |
756 | + self.assertEqual('mediawiki/bundle1', bundle.reference.charmworld_id) |
757 | + # The urlread function has been called two times: the first time |
758 | + # including both the basket and the bundle name, the second time |
759 | + # to retrieve the legacy bundle yaml, only including the basket. |
760 | + self.assertEqual(mock_urlread.call_count, 2) |
761 | + mock_urlread.assert_has_calls([ |
762 | + mock.call( |
763 | + settings.CHARMSTORE_API + |
764 | + 'bundle/mediawiki-bundle1/archive/bundle.yaml'), |
765 | + mock.call( |
766 | + settings.CHARMSTORE_API + |
767 | + 'bundle/mediawiki/archive/bundles.yaml.orig'), |
768 | + ]) |
769 | + |
770 | def test_charmworld_bundle_deprecation_warning(self): |
771 | # A deprecation warning is printed if the no longer supported |
772 | # charmworld bundle identifiers are used. |
773 | @@ -119,22 +162,73 @@ |
774 | |
775 | def test_charmworld_bundle_invalid_content(self): |
776 | # A ValueError is raised if the content associated with the given |
777 | - # charmworld URL are not valid. |
778 | - with self.patch_urlread(error=True): |
779 | - with self.assertRaises(IOError) as ctx: |
780 | + # charmworld URL is not valid. |
781 | + with self.patch_urlread(contents='exterminate!'): |
782 | + with self.assert_value_error('invalid YAML content: exterminate!'): |
783 | bundles.from_source('bundle:mediawiki/single') |
784 | - expected_error = ( |
785 | - 'cannot retrieve bundle from remote URL ' |
786 | - '{}bundle/mediawiki-single/archive/bundle.yaml: ' |
787 | - 'bad wolf'.format(settings.CHARMSTORE_API)) |
788 | - self.assertEqual(expected_error, bytes(ctx.exception)) |
789 | |
790 | def test_charmworld_bundle_connection_error(self): |
791 | # An IOError is raised if a connection problem is encountered while |
792 | # retrieving the charmworld bundle. |
793 | - with self.patch_urlread(contents='exterminate!'): |
794 | - with self.assert_value_error('invalid YAML content: exterminate!'): |
795 | - bundles.from_source('bundle:mediawiki/single') |
796 | + with self.patch_urlread(error=IOError('bad wolf')): |
797 | + with self.assertRaises(IOError) as ctx: |
798 | + bundles.from_source('bundle:mediawiki/single') |
799 | + expected_error = ( |
800 | + 'cannot communicate with the charm store at ' |
801 | + '{}bundle/mediawiki-single/archive/bundle.yaml: ' |
802 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
803 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
804 | + |
805 | + def test_charmworld_bundle_not_found_error(self): |
806 | + # An IOError is raised if a charmworld bundle is not found. |
807 | + error = netutils.NotFoundError('bad wolf') |
808 | + with self.patch_urlread(error=error) as mock_urlread: |
809 | + with self.assertRaises(IOError) as ctx: |
810 | + bundles.from_source('bundle:mediawiki/single') |
811 | + expected_error = ( |
812 | + 'charm store resource not found at ' |
813 | + '{}bundle/mediawiki/archive/bundles.yaml.orig: ' |
814 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
815 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
816 | + # The urlread function has been called two times: the first time |
817 | + # including both the basket and the bundle name, the second time |
818 | + # to retrieve the legacy bundle yaml, only including the basket. |
819 | + self.assertEqual(mock_urlread.call_count, 2) |
820 | + mock_urlread.assert_has_calls([ |
821 | + mock.call( |
822 | + settings.CHARMSTORE_API + |
823 | + 'bundle/mediawiki-single/archive/bundle.yaml'), |
824 | + mock.call( |
825 | + settings.CHARMSTORE_API + |
826 | + 'bundle/mediawiki/archive/bundles.yaml.orig'), |
827 | + ]) |
828 | + |
829 | + def test_charmworld_bundle_from_legacy_invalid_name(self): |
830 | + # A ValueError is raised if the given bundle name is not found in |
831 | + # the legacy basket. |
832 | + legacy_bundle_content = yaml.safe_dump({'scalable': self.bundle_data}) |
833 | + side_effect = [ |
834 | + # A first call urlread returns a not found error. |
835 | + netutils.NotFoundError('boo!'), |
836 | + # A second call succeeds. |
837 | + legacy_bundle_content, |
838 | + ] |
839 | + expected_error = 'bundle single not found, did you mean scalable?' |
840 | + with self.patch_urlread(error=side_effect) as mock_urlread: |
841 | + with self.assert_value_error(expected_error): |
842 | + bundles.from_source('bundle:django/single') |
843 | + # The urlread function has been called two times: the first time |
844 | + # including both the basket and the bundle name, the second time |
845 | + # to retrieve the legacy bundle yaml, only including the basket. |
846 | + self.assertEqual(mock_urlread.call_count, 2) |
847 | + mock_urlread.assert_has_calls([ |
848 | + mock.call( |
849 | + settings.CHARMSTORE_API + |
850 | + 'bundle/django-single/archive/bundle.yaml'), |
851 | + mock.call( |
852 | + settings.CHARMSTORE_API + |
853 | + 'bundle/django/archive/bundles.yaml.orig'), |
854 | + ]) |
855 | |
856 | def test_jujucharms_bundle(self): |
857 | # A bundle instance is properly returned from a jujucharms.com id. |
858 | @@ -173,21 +267,33 @@ |
859 | def test_jujucharms_bundle_invalid_content(self): |
860 | # A ValueError is raised if the content associated with the given |
861 | # jujucharms.com URL are not valid. |
862 | - with self.patch_urlread(error=True): |
863 | - with self.assertRaises(IOError) as ctx: |
864 | - bundles.from_source('django/42') |
865 | - expected_error = ( |
866 | - 'cannot retrieve bundle from remote URL ' |
867 | - '{}bundle/django-42/archive/bundle.yaml: ' |
868 | - 'bad wolf'.format(settings.CHARMSTORE_API)) |
869 | - self.assertEqual(expected_error, bytes(ctx.exception)) |
870 | + with self.patch_urlread(contents='exterminate!'): |
871 | + with self.assert_value_error('invalid YAML content: exterminate!'): |
872 | + bundles.from_source('wordpress-scalable') |
873 | |
874 | def test_jujucharms_bundle_connection_error(self): |
875 | # An IOError is raised if a connection problem is encountered while |
876 | # retrieving the jujucharms.com bundle. |
877 | - with self.patch_urlread(contents='exterminate!'): |
878 | - with self.assert_value_error('invalid YAML content: exterminate!'): |
879 | - bundles.from_source('wordpress-scalable') |
880 | + with self.patch_urlread(error=IOError('bad wolf')): |
881 | + with self.assertRaises(IOError) as ctx: |
882 | + bundles.from_source('django/42') |
883 | + expected_error = ( |
884 | + 'cannot communicate with the charm store at ' |
885 | + '{}bundle/django-42/archive/bundle.yaml: ' |
886 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
887 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
888 | + |
889 | + def test_jujucharms_bundle_not_found_error(self): |
890 | + # An IOError is raised if a connection problem is encountered while |
891 | + # retrieving the jujucharms.com bundle. |
892 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
893 | + with self.assertRaises(IOError) as ctx: |
894 | + bundles.from_source('django/42') |
895 | + expected_error = ( |
896 | + 'charm store resource not found at ' |
897 | + '{}bundle/django-42/archive/bundle.yaml: ' |
898 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
899 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
900 | |
901 | def test_local_file(self): |
902 | # A bundle instance can be created from a local file source. |
903 | @@ -248,11 +354,20 @@ |
904 | with self.assert_value_error(expected_error): |
905 | bundles.from_source(path, 'mybundle') |
906 | |
907 | + def test_local_file_legacy_bundle_single_bundle_name_not_found(self): |
908 | + # A ValueError is raised if a local file contains a legacy version 3 |
909 | + # YAML content with one bundle defined and the provided bundle name |
910 | + # does not match. |
911 | + path = self.make_bundle_file({'scalable': self.bundle_data}) |
912 | + expected_error = 'bundle mybundle not found, did you mean scalable?' |
913 | + with self.assert_value_error(expected_error): |
914 | + bundles.from_source(path, 'mybundle') |
915 | + |
916 | def test_local_file_legacy_bundle_invalid_bundle_content(self): |
917 | # A ValueError is raised if a local file contains an invalid legacy |
918 | # version 3 content. |
919 | path = self.make_bundle_file({'bundle': '42'}) |
920 | - expected_error = "unable to retrieve bundle services: '42'" |
921 | + expected_error = "invalid YAML content: 42" |
922 | with self.assert_value_error(expected_error): |
923 | bundles.from_source(path, 'bundle') |
924 | |
925 | @@ -280,7 +395,7 @@ |
926 | def test_remote_url_not_reachable(self): |
927 | # An IOError is raised if a network problem is encountered while |
928 | # trying to reach the remote URL. |
929 | - with self.patch_urlread(error=True): |
930 | + with self.patch_urlread(error=IOError('bad wolf')): |
931 | with self.assertRaises(IOError) as ctx: |
932 | bundles.from_source('https://1.2.3.4') |
933 | expected_error = ( |
934 | @@ -317,6 +432,16 @@ |
935 | with self.assert_value_error(expected_error): |
936 | bundles.from_source('http://1.2.3.4', 'no-such') |
937 | |
938 | + def test_remote_url_legacy_bundle_single_bundle_name_not_found(self): |
939 | + # A ValueError is raised if a remote URL contains a legacy version 3 |
940 | + # YAML content with one bundle defined and the provided bundle name |
941 | + # does not match. |
942 | + legacy_bundle_content = yaml.safe_dump({'scalable': self.bundle_data}) |
943 | + expected_error = 'bundle no-such not found, did you mean scalable?' |
944 | + with self.patch_urlread(contents=legacy_bundle_content): |
945 | + with self.assert_value_error(expected_error): |
946 | + bundles.from_source('http://1.2.3.4', 'no-such') |
947 | + |
948 | def test_remote_url_legacy_bundle_invalid_bundle_content(self): |
949 | # A ValueError is raised if a remote URL contains an invalid legacy |
950 | # version 3 content. |
951 | @@ -399,6 +524,22 @@ |
952 | self.assertEqual(self.bundle_data, data) |
953 | |
954 | |
955 | +class TestIsLegacyBundle(helpers.BundleFileTestsMixin, unittest.TestCase): |
956 | + |
957 | + def test_v4_bundle(self): |
958 | + # False is returned for new-style version 4 bundles. |
959 | + self.assertFalse(bundles.is_legacy_bundle(self.bundle_data)) |
960 | + |
961 | + def test_legacy_bundle(self): |
962 | + # True is returned for legacy bundles. |
963 | + tests = ( |
964 | + self.legacy_bundle_data, |
965 | + {'services': {'services': {}}}, |
966 | + ) |
967 | + for data in tests: |
968 | + self.assertTrue(bundles.is_legacy_bundle(data)) |
969 | + |
970 | + |
971 | class TestValidate( |
972 | helpers.BundleFileTestsMixin, helpers.ValueErrorTestsMixin, |
973 | unittest.TestCase): |
974 | |
975 | === modified file 'quickstart/tests/models/test_references.py' |
976 | --- quickstart/tests/models/test_references.py 2015-02-26 19:46:48 +0000 |
977 | +++ quickstart/tests/models/test_references.py 2015-03-10 10:19:09 +0000 |
978 | @@ -318,43 +318,6 @@ |
979 | self.assertEqual(expected_ref, ref) |
980 | |
981 | |
982 | -class TestReferenceFromCharmworldUrl( |
983 | - helpers.ValueErrorTestsMixin, unittest.TestCase): |
984 | - |
985 | - def test_invalid_form(self): |
986 | - # A ValueError is raised if the URL is not valid. |
987 | - expected_error = 'invalid bundle URL: bad wolf' |
988 | - with self.assert_value_error(expected_error): |
989 | - references.Reference.from_charmworld_url('bad wolf') |
990 | - |
991 | - def test_success(self): |
992 | - # A reference is correctly created from a charmworld identifier. |
993 | - tests = ( |
994 | - ('bundle:~myuser/wordpress/42/single', |
995 | - make_reference(series='bundle', name='wordpress-single')), |
996 | - ('bundle:~myuser/wordpress/single', |
997 | - make_reference(series='bundle', name='wordpress-single', |
998 | - revision=None)), |
999 | - ('bundle:wordpress/42/single', |
1000 | - make_reference(user='', series='bundle', |
1001 | - name='wordpress-single')), |
1002 | - ('bundle:wordpress/single', |
1003 | - make_reference(user='', series='bundle', name='wordpress-single', |
1004 | - revision=None)), |
1005 | - ) |
1006 | - for url, expected_ref in tests: |
1007 | - ref = references.Reference.from_charmworld_url(url) |
1008 | - self.assertEqual(expected_ref, ref) |
1009 | - |
1010 | - def test_charmworld_id(self): |
1011 | - # The charmworld id is properly set when parsing charmworld URLs. |
1012 | - # XXX frankban 2015-02-26: remove this test once we get rid of the |
1013 | - # charmworld id concept. |
1014 | - ref = references.Reference.from_charmworld_url( |
1015 | - 'bundle:wordpress/single') |
1016 | - self.assertEqual('wordpress/single', ref.charmworld_id) |
1017 | - |
1018 | - |
1019 | class TestReferenceFromJujucharmsUrl( |
1020 | helpers.ValueErrorTestsMixin, unittest.TestCase): |
1021 | |
1022 | |
1023 | === modified file 'quickstart/tests/test_app.py' |
1024 | --- quickstart/tests/test_app.py 2015-02-26 19:46:48 +0000 |
1025 | +++ quickstart/tests/test_app.py 2015-03-10 10:19:09 +0000 |
1026 | @@ -930,12 +930,11 @@ |
1027 | env.get_status.side_effect = side_effect |
1028 | return env |
1029 | |
1030 | - def patch_get_charm_url(self, return_value=None, side_effect=None): |
1031 | - """Patch the get_charm_url helper function.""" |
1032 | - mock_get_charm_url = mock.Mock( |
1033 | + def patch_resolve(self, return_value=None, side_effect=None): |
1034 | + """Patch the charmstore.resolve function.""" |
1035 | + mock_resolve = mock.Mock( |
1036 | return_value=return_value, side_effect=side_effect) |
1037 | - return mock.patch( |
1038 | - 'quickstart.netutils.get_charm_url', mock_get_charm_url) |
1039 | + return mock.patch('quickstart.charmstore.resolve', mock_resolve) |
1040 | |
1041 | def assert_reference_equal(self, expected_url, ref): |
1042 | """Ensure the given reference points to the expected URL.""" |
1043 | @@ -954,8 +953,8 @@ |
1044 | env_type = 'ec2' |
1045 | bootstrap_node_series = 'trusty' |
1046 | check_preexisting = False |
1047 | - with self.patch_get_charm_url( |
1048 | - return_value='cs:trusty/juju-gui-42') as mock_get_charm_url: |
1049 | + with self.patch_resolve( |
1050 | + return_value='cs:trusty/juju-gui-42') as mock_resolve: |
1051 | ref, machine, service_data, unit_data = app.check_environment( |
1052 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1053 | check_preexisting) |
1054 | @@ -964,7 +963,8 @@ |
1055 | # The charm URL has been retrieved from the charm store API based on |
1056 | # the current bootstrap node series. |
1057 | self.assert_reference_equal('cs:trusty/juju-gui-42', ref) |
1058 | - mock_get_charm_url.assert_called_once_with(bootstrap_node_series) |
1059 | + mock_resolve.assert_called_once_with( |
1060 | + settings.JUJU_GUI_CHARM_NAME, series=bootstrap_node_series) |
1061 | # Since the bootstrap node series is supported by the GUI charm, the |
1062 | # GUI unit can be deployed to machine 0. |
1063 | self.assertEqual('0', machine) |
1064 | @@ -989,8 +989,8 @@ |
1065 | env_type = 'ec2' |
1066 | bootstrap_node_series = 'precise' |
1067 | check_preexisting = True |
1068 | - with self.patch_get_charm_url( |
1069 | - return_value='cs:precise/juju-gui-42') as mock_get_charm_url: |
1070 | + with self.patch_resolve( |
1071 | + return_value='cs:precise/juju-gui-42') as mock_resolve: |
1072 | ref, machine, service_data, unit_data = app.check_environment( |
1073 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1074 | check_preexisting) |
1075 | @@ -999,7 +999,8 @@ |
1076 | # The charm URL has been retrieved from the charm store API based on |
1077 | # the current bootstrap node series. |
1078 | self.assert_reference_equal('cs:precise/juju-gui-42', ref) |
1079 | - mock_get_charm_url.assert_called_once_with(bootstrap_node_series) |
1080 | + mock_resolve.assert_called_once_with( |
1081 | + settings.JUJU_GUI_CHARM_NAME, series=bootstrap_node_series) |
1082 | # Since the bootstrap node series is supported by the GUI charm, the |
1083 | # GUI unit can be deployed to machine 0. |
1084 | self.assertEqual('0', machine) |
1085 | @@ -1022,7 +1023,7 @@ |
1086 | env_type = 'ec2' |
1087 | bootstrap_node_series = 'precise' |
1088 | check_preexisting = True |
1089 | - with self.patch_get_charm_url() as mock_get_charm_url: |
1090 | + with self.patch_resolve() as mock_resolve: |
1091 | ref, machine, service_data, unit_data = app.check_environment( |
1092 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1093 | check_preexisting) |
1094 | @@ -1030,7 +1031,7 @@ |
1095 | env.get_status.assert_called_once_with() |
1096 | # The charm URL has been retrieved from the environment. |
1097 | self.assert_reference_equal('cs:precise/juju-gui-47', ref) |
1098 | - self.assertFalse(mock_get_charm_url.called) |
1099 | + self.assertFalse(mock_resolve.called) |
1100 | # Since the bootstrap node series is supported by the GUI charm, the |
1101 | # GUI unit can be safely deployed to machine 0. |
1102 | self.assertEqual('0', machine) |
1103 | @@ -1046,15 +1047,16 @@ |
1104 | env_type = 'ec2' |
1105 | bootstrap_node_series = 'saucy' |
1106 | check_preexisting = False |
1107 | - with self.patch_get_charm_url( |
1108 | - return_value='cs:trusty/juju-gui-42') as mock_get_charm_url: |
1109 | + with self.patch_resolve( |
1110 | + return_value='cs:trusty/juju-gui-42') as mock_resolve: |
1111 | ref, machine, service_data, unit_data = app.check_environment( |
1112 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1113 | check_preexisting) |
1114 | # The charm URL has been retrieved from the charm store API using the |
1115 | # most recent supported series. |
1116 | self.assert_reference_equal('cs:trusty/juju-gui-42', ref) |
1117 | - mock_get_charm_url.assert_called_once_with('trusty') |
1118 | + mock_resolve.assert_called_once_with( |
1119 | + settings.JUJU_GUI_CHARM_NAME, series='trusty') |
1120 | # The Juju GUI unit cannot be deployed to saucy machine 0. |
1121 | self.assertIsNone(machine) |
1122 | # Ensure the function output makes sense. |
1123 | @@ -1072,7 +1074,7 @@ |
1124 | env_type = 'local' |
1125 | bootstrap_node_series = 'trusty' |
1126 | check_preexisting = False |
1127 | - with self.patch_get_charm_url(return_value='cs:trusty/juju-gui-42'): |
1128 | + with self.patch_resolve(return_value='cs:trusty/juju-gui-42'): |
1129 | ref, machine, service_data, unit_data = app.check_environment( |
1130 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1131 | check_preexisting) |
1132 | @@ -1089,7 +1091,7 @@ |
1133 | env_type = 'azure' |
1134 | bootstrap_node_series = 'trusty' |
1135 | check_preexisting = False |
1136 | - with self.patch_get_charm_url(return_value='cs:trusty/juju-gui-42'): |
1137 | + with self.patch_resolve(return_value='cs:trusty/juju-gui-42'): |
1138 | _, machine, _, _ = app.check_environment( |
1139 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1140 | check_preexisting) |
1141 | @@ -1103,7 +1105,7 @@ |
1142 | env_type = 'ec2' |
1143 | bootstrap_node_series = 'precise' |
1144 | check_preexisting = False |
1145 | - with self.patch_get_charm_url(side_effect=IOError('boo!')): |
1146 | + with self.patch_resolve(side_effect=IOError('boo!')): |
1147 | ref, machine, service_data, unit_data = app.check_environment( |
1148 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1149 | check_preexisting) |
1150 | @@ -1121,7 +1123,7 @@ |
1151 | env_type = 'ec2' |
1152 | bootstrap_node_series = 'saucy' |
1153 | check_preexisting = False |
1154 | - with self.patch_get_charm_url(side_effect=IOError('boo!')): |
1155 | + with self.patch_resolve(side_effect=IOError('boo!')): |
1156 | ref, machine, service_data, unit_data = app.check_environment( |
1157 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1158 | check_preexisting) |
1159 | @@ -1138,13 +1140,13 @@ |
1160 | env_type = 'ec2' |
1161 | bootstrap_node_series = 'trusty' |
1162 | check_preexisting = False |
1163 | - with self.patch_get_charm_url() as mock_get_charm_url: |
1164 | + with self.patch_resolve() as mock_resolve: |
1165 | ref, machine, service_data, unit_data = app.check_environment( |
1166 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1167 | check_preexisting) |
1168 | - # There is no need to call the charmword API if the charm URL is |
1169 | + # There is no need to call the charm store API if the charm URL is |
1170 | # provided by the user. |
1171 | - self.assertFalse(mock_get_charm_url.called) |
1172 | + self.assertFalse(mock_resolve.called) |
1173 | # The provided charm URL has been correctly returned. |
1174 | self.assert_reference_equal(charm_url, ref) |
1175 | # Since the provided charm series is trusty, the charm itself can be |
1176 | @@ -1165,13 +1167,13 @@ |
1177 | env_type = 'ec2' |
1178 | bootstrap_node_series = 'precise' |
1179 | check_preexisting = False |
1180 | - with self.patch_get_charm_url() as mock_get_charm_url: |
1181 | + with self.patch_resolve() as mock_resolve: |
1182 | ref, machine, service_data, unit_data = app.check_environment( |
1183 | env, 'my-gui', charm_url, env_type, bootstrap_node_series, |
1184 | check_preexisting) |
1185 | - # There is no need to call the charmword API if the charm URL is |
1186 | + # There is no need to call the charm store API if the charm URL is |
1187 | # provided by the user. |
1188 | - self.assertFalse(mock_get_charm_url.called) |
1189 | + self.assertFalse(mock_resolve.called) |
1190 | # The provided charm URL has been correctly returned. |
1191 | self.assert_reference_equal(charm_url, ref) |
1192 | # Since the provided charm series is not precise, the charm must be |
1193 | @@ -1666,7 +1668,9 @@ |
1194 | # XXX frankban 2015-02-26: remove this test once we get rid of the |
1195 | # charmworld id concept. |
1196 | env = mock.Mock() |
1197 | - ref = references.Reference.from_charmworld_url('bundle:django/single') |
1198 | + ref = references.Reference.from_fully_qualified_url( |
1199 | + 'cs:bundle/django-single-42') |
1200 | + ref.charmworld_id = 'django/single' |
1201 | bundle = bundles.Bundle(self.bundle_data, reference=ref) |
1202 | app.deploy_bundle(env, bundle) |
1203 | env.deploy_bundle.assert_called_once_with( |
1204 | |
1205 | === added file 'quickstart/tests/test_charmstore.py' |
1206 | --- quickstart/tests/test_charmstore.py 1970-01-01 00:00:00 +0000 |
1207 | +++ quickstart/tests/test_charmstore.py 2015-03-10 10:19:09 +0000 |
1208 | @@ -0,0 +1,313 @@ |
1209 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
1210 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
1211 | +# Copyright (C) 2015 Canonical Ltd. |
1212 | +# |
1213 | +# This program is free software: you can redistribute it and/or modify it under |
1214 | +# the terms of the GNU Affero General Public License version 3, as published by |
1215 | +# the Free Software Foundation. |
1216 | +# |
1217 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
1218 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
1219 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
1220 | +# Affero General Public License for more details. |
1221 | +# |
1222 | +# You should have received a copy of the GNU Affero General Public License |
1223 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
1224 | + |
1225 | +"""Tests for the Juju Quickstart charm store communication utilities.""" |
1226 | + |
1227 | +from __future__ import unicode_literals |
1228 | + |
1229 | +import unittest |
1230 | + |
1231 | +import json |
1232 | + |
1233 | +from quickstart import ( |
1234 | + charmstore, |
1235 | + netutils, |
1236 | + settings, |
1237 | +) |
1238 | +from quickstart.models import references |
1239 | +from quickstart.tests import helpers |
1240 | + |
1241 | + |
1242 | +class TestNotFoundError(unittest.TestCase): |
1243 | + |
1244 | + def test_error_message(self): |
1245 | + # The error message can be properly retrieved. |
1246 | + err = charmstore.NotFoundError(b'bad wolf') |
1247 | + self.assertEqual('bad wolf', bytes(err)) |
1248 | + |
1249 | + |
1250 | +class TestGet(helpers.UrlReadTestsMixin, unittest.TestCase): |
1251 | + |
1252 | + def test_success(self): |
1253 | + # A GET request to the charm store is correctly performed. |
1254 | + with self.patch_urlread(contents='ok') as mock_urlread: |
1255 | + content = charmstore.get('my/path') |
1256 | + self.assertEqual('ok', content) |
1257 | + mock_urlread.assert_called_once_with( |
1258 | + settings.CHARMSTORE_API + 'my/path') |
1259 | + |
1260 | + def test_success_leading_slash(self): |
1261 | + # The resulting URL is correctly formatted. |
1262 | + with self.patch_urlread() as mock_urlread: |
1263 | + charmstore.get('/path/') |
1264 | + mock_urlread.assert_called_once_with(settings.CHARMSTORE_API + 'path/') |
1265 | + |
1266 | + def test_io_error(self): |
1267 | + # An IOError is raised if a problem is encountered while connecting to |
1268 | + # the charm store. |
1269 | + with self.patch_urlread(error=IOError('bad wolf')): |
1270 | + with self.assertRaises(IOError) as ctx: |
1271 | + charmstore.get('/') |
1272 | + expected_error = ( |
1273 | + 'cannot communicate with the charm store at ' |
1274 | + '{}: bad wolf'.format(settings.CHARMSTORE_API)) |
1275 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1276 | + |
1277 | + def test_not_found_error(self): |
1278 | + # A charmstore.NotFoundError is raised if the request returns a 404 not |
1279 | + # found response. |
1280 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
1281 | + with self.assertRaises(charmstore.NotFoundError) as ctx: |
1282 | + charmstore.get('/no/such') |
1283 | + expected_error = ( |
1284 | + 'charm store resource not found at ' |
1285 | + '{}no/such: bad wolf'.format(settings.CHARMSTORE_API)) |
1286 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1287 | + |
1288 | + |
1289 | +class TestGetReference(helpers.UrlReadTestsMixin, unittest.TestCase): |
1290 | + |
1291 | + def test_success(self): |
1292 | + # A GET request to the charm store is correctly performed using the |
1293 | + # given charm or bundle reference. |
1294 | + ref = references.Reference.from_jujucharms_url('mediawiki-single') |
1295 | + with self.patch_urlread(contents='hash') as mock_urlread: |
1296 | + content = charmstore.get_reference(ref, '/meta/hash') |
1297 | + self.assertEqual('hash', content) |
1298 | + mock_urlread.assert_called_once_with( |
1299 | + settings.CHARMSTORE_API + 'bundle/mediawiki-single/meta/hash') |
1300 | + |
1301 | + def test_success_without_leading_slash(self): |
1302 | + # The resulting URL is correctly formatted when the static path does |
1303 | + # not include a leading slash. |
1304 | + ref = references.Reference.from_jujucharms_url('django/trusty/42') |
1305 | + with self.patch_urlread() as mock_urlread: |
1306 | + charmstore.get_reference(ref, 'expand-id') |
1307 | + mock_urlread.assert_called_once_with( |
1308 | + settings.CHARMSTORE_API + 'trusty/django-42/expand-id') |
1309 | + |
1310 | + def test_io_error(self): |
1311 | + # An IOError is raised if a problem is encountered while connecting to |
1312 | + # the charm store. |
1313 | + ref = references.Reference.from_jujucharms_url('django/trusty') |
1314 | + with self.patch_urlread(error=IOError('bad wolf')): |
1315 | + with self.assertRaises(IOError) as ctx: |
1316 | + charmstore.get_reference(ref, 'meta/id') |
1317 | + expected_error = ( |
1318 | + 'cannot communicate with the charm store at ' |
1319 | + '{}trusty/django/meta/id: ' |
1320 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1321 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1322 | + |
1323 | + def test_not_found_error(self): |
1324 | + # A charmstore.NotFoundError is raised if the reference is not found |
1325 | + # in the charm store. |
1326 | + ref = references.Reference.from_jujucharms_url('django') |
1327 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
1328 | + with self.assertRaises(charmstore.NotFoundError) as ctx: |
1329 | + charmstore.get_reference(ref, '/no/such') |
1330 | + expected_error = ( |
1331 | + 'charm store resource not found at ' |
1332 | + '{}bundle/django/no/such: ' |
1333 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1334 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1335 | + |
1336 | + |
1337 | +class TestResolve(helpers.UrlReadTestsMixin, unittest.TestCase): |
1338 | + |
1339 | + contents = json.dumps({ |
1340 | + 'Id': 'cs:trusty/juju-gui-42', |
1341 | + 'Series': 'trusty', |
1342 | + 'Name': 'juju-gui', |
1343 | + 'Revision': 42, |
1344 | + }) |
1345 | + |
1346 | + def test_resolved(self): |
1347 | + # The resolved entity id is correctly returned. |
1348 | + with self.patch_urlread(contents=self.contents) as mock_urlread: |
1349 | + entity_id = charmstore.resolve('juju-gui') |
1350 | + self.assertEqual('cs:trusty/juju-gui-42', entity_id) |
1351 | + mock_urlread.assert_called_once_with( |
1352 | + settings.CHARMSTORE_API + 'juju-gui/meta/id') |
1353 | + |
1354 | + def test_resolved_with_series(self): |
1355 | + # The resolved entity id is correctly returned when the entity series |
1356 | + # is specified. |
1357 | + with self.patch_urlread(contents=self.contents) as mock_urlread: |
1358 | + entity_id = charmstore.resolve('django', series='vivid') |
1359 | + self.assertEqual('cs:trusty/juju-gui-42', entity_id) |
1360 | + mock_urlread.assert_called_once_with( |
1361 | + settings.CHARMSTORE_API + 'vivid/django/meta/id') |
1362 | + |
1363 | + def test_io_error(self): |
1364 | + # IOErrors are properly propagated. |
1365 | + with self.patch_urlread(error=IOError('bad wolf')): |
1366 | + with self.assertRaises(IOError) as ctx: |
1367 | + charmstore.resolve('django') |
1368 | + expected_error = ( |
1369 | + 'cannot communicate with the charm store at ' |
1370 | + '{}django/meta/id: bad wolf'.format(settings.CHARMSTORE_API)) |
1371 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1372 | + |
1373 | + def test_not_found_error(self): |
1374 | + # Not found errors are properly propagated. |
1375 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
1376 | + with self.assertRaises(charmstore.NotFoundError) as ctx: |
1377 | + charmstore.resolve('django', series='trusty') |
1378 | + expected_error = ( |
1379 | + 'charm store resource not found at ' |
1380 | + '{}trusty/django/meta/id: ' |
1381 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1382 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1383 | + |
1384 | + def test_value_error(self): |
1385 | + # A ValueError is raised if the API response is not valid. |
1386 | + contents = json.dumps({'invalid': {}}) |
1387 | + with self.patch_urlread(contents=contents): |
1388 | + with self.assertRaises(ValueError) as ctx: |
1389 | + charmstore.resolve('juju-gui', series='trusty') |
1390 | + self.assertEqual( |
1391 | + 'unable to resolve entity id trusty/juju-gui', |
1392 | + bytes(ctx.exception)) |
1393 | + |
1394 | + |
1395 | +class TestGetBundleData( |
1396 | + helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin, |
1397 | + unittest.TestCase): |
1398 | + |
1399 | + def test_data_retrieved(self): |
1400 | + # The bundle data is correctly retrieved and parsed. |
1401 | + ref = references.Reference.from_jujucharms_url('django/42') |
1402 | + with self.patch_urlread(contents=self.bundle_content) as mock_urlread: |
1403 | + data = charmstore.get_bundle_data(ref) |
1404 | + self.assertEqual(self.bundle_data, data) |
1405 | + mock_urlread.assert_called_once_with( |
1406 | + settings.CHARMSTORE_API + 'bundle/django-42/archive/bundle.yaml') |
1407 | + |
1408 | + def test_io_error(self): |
1409 | + # IOErrors are properly propagated. |
1410 | + ref = references.Reference.from_jujucharms_url('mediawiki-single') |
1411 | + with self.patch_urlread(error=IOError('bad wolf')): |
1412 | + with self.assertRaises(IOError) as ctx: |
1413 | + charmstore.get_bundle_data(ref) |
1414 | + expected_error = ( |
1415 | + 'cannot communicate with the charm store at ' |
1416 | + '{}bundle/mediawiki-single/archive/bundle.yaml: ' |
1417 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1418 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1419 | + |
1420 | + def test_not_found_error(self): |
1421 | + # Not found errors are properly propagated. |
1422 | + ref = references.Reference.from_jujucharms_url('no-such') |
1423 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
1424 | + with self.assertRaises(charmstore.NotFoundError) as ctx: |
1425 | + charmstore.get_bundle_data(ref) |
1426 | + expected_error = ( |
1427 | + 'charm store resource not found at ' |
1428 | + '{}bundle/no-such/archive/bundle.yaml: ' |
1429 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1430 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1431 | + |
1432 | + def test_value_error_invalid_data(self): |
1433 | + # A ValueError is raised if the API response is not valid. |
1434 | + ref = references.Reference.from_jujucharms_url('u/who/django') |
1435 | + with self.patch_urlread(contents='invalid: data:'): |
1436 | + with self.assertRaises(ValueError) as ctx: |
1437 | + charmstore.get_bundle_data(ref) |
1438 | + self.assertIn( |
1439 | + 'unable to parse the bundle content', bytes(ctx.exception)) |
1440 | + |
1441 | + def test_value_error_not_a_bundle(self): |
1442 | + # A ValueError is raised if the API response is not valid. |
1443 | + ref = references.Reference.from_jujucharms_url('django/trusty/47') |
1444 | + with self.assertRaises(ValueError) as ctx: |
1445 | + charmstore.get_bundle_data(ref) |
1446 | + self.assertEqual( |
1447 | + 'expected a bundle, provided charm cs:trusty/django-47', |
1448 | + bytes(ctx.exception)) |
1449 | + |
1450 | + |
1451 | +class TestGetLegacyBundleData( |
1452 | + helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin, |
1453 | + unittest.TestCase): |
1454 | + |
1455 | + def test_data_retrieved(self): |
1456 | + # The legacy bundle data is correctly retrieved and parsed. |
1457 | + ref = references.Reference.from_jujucharms_url('u/who/django/42') |
1458 | + contents = self.legacy_bundle_content |
1459 | + with self.patch_urlread(contents=contents) as mock_urlread: |
1460 | + data = charmstore.get_legacy_bundle_data(ref) |
1461 | + self.assertEqual(self.legacy_bundle_data, data) |
1462 | + mock_urlread.assert_called_once_with( |
1463 | + settings.CHARMSTORE_API + |
1464 | + '~who/bundle/django-42/archive/bundles.yaml.orig') |
1465 | + |
1466 | + def test_io_error(self): |
1467 | + # IOErrors are properly propagated. |
1468 | + ref = references.Reference.from_jujucharms_url('mediawiki-single') |
1469 | + with self.patch_urlread(error=IOError('bad wolf')): |
1470 | + with self.assertRaises(IOError) as ctx: |
1471 | + charmstore.get_legacy_bundle_data(ref) |
1472 | + expected_error = ( |
1473 | + 'cannot communicate with the charm store at ' |
1474 | + '{}bundle/mediawiki-single/archive/bundles.yaml.orig: ' |
1475 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1476 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1477 | + |
1478 | + def test_not_found_error(self): |
1479 | + # Not found errors are properly propagated. |
1480 | + ref = references.Reference.from_jujucharms_url('no-such') |
1481 | + with self.patch_urlread(error=netutils.NotFoundError('bad wolf')): |
1482 | + with self.assertRaises(charmstore.NotFoundError) as ctx: |
1483 | + charmstore.get_legacy_bundle_data(ref) |
1484 | + expected_error = ( |
1485 | + 'charm store resource not found at ' |
1486 | + '{}bundle/no-such/archive/bundles.yaml.orig: ' |
1487 | + 'bad wolf'.format(settings.CHARMSTORE_API)) |
1488 | + self.assertEqual(expected_error, bytes(ctx.exception)) |
1489 | + |
1490 | + def test_value_error_invalid_data(self): |
1491 | + # A ValueError is raised if the API response is not valid. |
1492 | + ref = references.Reference.from_jujucharms_url('u/who/django') |
1493 | + with self.patch_urlread(contents='invalid: data:'): |
1494 | + with self.assertRaises(ValueError) as ctx: |
1495 | + charmstore.get_legacy_bundle_data(ref) |
1496 | + self.assertIn( |
1497 | + 'unable to parse the bundle content', bytes(ctx.exception)) |
1498 | + |
1499 | + def test_value_error_not_a_bundle(self): |
1500 | + # A ValueError is raised if the API response is not valid. |
1501 | + ref = references.Reference.from_jujucharms_url('django/trusty/47') |
1502 | + with self.assertRaises(ValueError) as ctx: |
1503 | + charmstore.get_legacy_bundle_data(ref) |
1504 | + self.assertEqual( |
1505 | + 'expected a bundle, provided charm cs:trusty/django-47', |
1506 | + bytes(ctx.exception)) |
1507 | + |
1508 | + |
1509 | +class TestLoadBundleYaml(helpers.BundleFileTestsMixin, unittest.TestCase): |
1510 | + |
1511 | + def test_valid_bundle_content(self): |
1512 | + # The bundle content is correctly loaded. |
1513 | + data = charmstore.load_bundle_yaml(self.bundle_content) |
1514 | + self.assertEqual(self.bundle_data, data) |
1515 | + |
1516 | + def test_invalid_bundle_content(self): |
1517 | + # A ValueError is raised if the bundle content is not valid. |
1518 | + with self.assertRaises(ValueError) as ctx: |
1519 | + charmstore.load_bundle_yaml('invalid: content:') |
1520 | + self.assertIn( |
1521 | + 'unable to parse the bundle content', bytes(ctx.exception)) |
1522 | |
1523 | === modified file 'quickstart/tests/test_netutils.py' |
1524 | --- quickstart/tests/test_netutils.py 2015-01-12 15:07:51 +0000 |
1525 | +++ quickstart/tests/test_netutils.py 2015-03-10 10:19:09 +0000 |
1526 | @@ -19,7 +19,6 @@ |
1527 | from __future__ import unicode_literals |
1528 | |
1529 | import httplib |
1530 | -import json |
1531 | import socket |
1532 | import unittest |
1533 | import urllib2 |
1534 | @@ -103,42 +102,12 @@ |
1535 | netutils.check_listening('1.2.3.4:17070') |
1536 | |
1537 | |
1538 | -class TestGetCharmUrl(helpers.UrlReadTestsMixin, unittest.TestCase): |
1539 | - |
1540 | - def test_charm_url(self): |
1541 | - # The Juju GUI charm URL is correctly returned. |
1542 | - contents = json.dumps({ |
1543 | - 'Id': 'cs:trusty/juju-gui-42', |
1544 | - 'Series': 'trusty', |
1545 | - 'Name': 'juju-gui', |
1546 | - 'Revision': 42, |
1547 | - }) |
1548 | - with self.patch_urlread(contents=contents) as mock_urlread: |
1549 | - charm_url = netutils.get_charm_url('trusty') |
1550 | - self.assertEqual('cs:trusty/juju-gui-42', charm_url) |
1551 | - mock_urlread.assert_called_once_with( |
1552 | - 'https://api.jujucharms.com/charmstore/v4/trusty/juju-gui/meta/id') |
1553 | - |
1554 | - def test_io_error(self): |
1555 | - # IOErrors are properly propagated. |
1556 | - with self.patch_urlread(error=True) as mock_urlread: |
1557 | - with self.assertRaises(IOError) as context_manager: |
1558 | - netutils.get_charm_url('precise') |
1559 | - mock_urlread.assert_called_once_with( |
1560 | - 'https://api.jujucharms.com/charmstore/v4/precise/juju-gui/meta/id' |
1561 | - ) |
1562 | - self.assertEqual('bad wolf', bytes(context_manager.exception)) |
1563 | - |
1564 | - def test_value_error(self): |
1565 | - # A ValueError is raised if the API response is not valid. |
1566 | - contents = json.dumps({'invalid': {}}) |
1567 | - with self.patch_urlread(contents=contents) as mock_urlread: |
1568 | - with self.assertRaises(ValueError) as context_manager: |
1569 | - netutils.get_charm_url('trusty') |
1570 | - mock_urlread.assert_called_once_with( |
1571 | - 'https://api.jujucharms.com/charmstore/v4/trusty/juju-gui/meta/id') |
1572 | - self.assertEqual( |
1573 | - 'unable to find the charm URL', bytes(context_manager.exception)) |
1574 | +class TestNotFoundError(unittest.TestCase): |
1575 | + |
1576 | + def test_error_message(self): |
1577 | + # The error message can be properly retrieved. |
1578 | + err = netutils.NotFoundError(b'bad wolf') |
1579 | + self.assertEqual('bad wolf', bytes(err)) |
1580 | |
1581 | |
1582 | class TestUrlread(unittest.TestCase): |
1583 | @@ -191,7 +160,14 @@ |
1584 | self.assertIsInstance(contents, unicode) |
1585 | mock_urlopen.assert_called_once_with('http://example.com/path/') |
1586 | |
1587 | - def test_errors(self): |
1588 | + def test_logging(self): |
1589 | + # The request URL is properly logged. |
1590 | + expected_log = 'sending HTTP GET request to http://example.com/path/' |
1591 | + with helpers.assert_logs([expected_log], level='debug'): |
1592 | + with self.patch_urlopen(): |
1593 | + netutils.urlread('http://example.com/path/') |
1594 | + |
1595 | + def test_io_errors(self): |
1596 | # An IOError is raised if an error occurs connecting to the API. |
1597 | errors = { |
1598 | 'httplib HTTPException': httplib.HTTPException, |
1599 | @@ -201,7 +177,16 @@ |
1600 | for message, exception_class in errors.items(): |
1601 | exception = exception_class(message) |
1602 | with self.patch_urlopen(error=exception) as mock_urlopen: |
1603 | - with self.assertRaises(IOError) as context_manager: |
1604 | + with self.assertRaises(IOError) as ctx: |
1605 | netutils.urlread('http://example.com/path/') |
1606 | mock_urlopen.assert_called_once_with('http://example.com/path/') |
1607 | - self.assertEqual(message, bytes(context_manager.exception)) |
1608 | + self.assertEqual(message, bytes(ctx.exception)) |
1609 | + |
1610 | + def test_not_found_error(self): |
1611 | + # A netutils.NotFoundError is raised if the request returns a 404 not |
1612 | + # found response. |
1613 | + exception = urllib2.HTTPError('url', 404, 'bad wolf', None, None) |
1614 | + with self.patch_urlopen(error=exception): |
1615 | + with self.assertRaises(netutils.NotFoundError) as ctx: |
1616 | + netutils.urlread('http://example.com/path/') |
1617 | + self.assertEqual('HTTP Error 404: bad wolf', bytes(ctx.exception)) |
1618 | |
1619 | === modified file 'quickstart/utils.py' |
1620 | --- quickstart/utils.py 2015-02-09 12:34:33 +0000 |
1621 | +++ quickstart/utils.py 2015-03-10 10:19:09 +0000 |
1622 | @@ -88,7 +88,7 @@ |
1623 | The banner is returned as a string, e.g.: |
1624 | |
1625 | # This file has been generated by juju quickstart v0.42.0 |
1626 | - # in date 2013-12-31 23:59:00 UTC. |
1627 | + # at 2013-12-31 23:59:00 UTC. |
1628 | """ |
1629 | now = datetime.datetime.utcnow() |
1630 | formatted_date = now.isoformat(sep=b' ').split('.')[0] |
Reviewers: mp+252353_ code.launchpad. net,
Message:
Please take a look.
Description:
Fix the old-style bundle regression.
This branch fixes /bugs.launchpad .net/juju- quickstart/ +bug/1429129 name}-{ bundle- name} for
https:/
In essence, legacy bundles are converted by the
ingestion process like the following:
- if a basked includes multiple bundles, the resulting
v4 bundle name is {basket-
each {bundle-name};
- if a basket only includes one bundle, the v4 bundle
is just {basket-name}.
Previously quickstart always assumed the former: this
branch adds a check for the latter before exiting with
an error.
This branch also introduces a charmstore module in
quickstart. All the interactions between quickstart
and the charm store are now collected in this module.
As part of this refactoring, quickstart is now able
to distinguish HTTP 404 errors and all the other
generic IOErrors that can be raised when connecting
to the network.
Also simplified the logging format and bootstrap logging
earlier in the application execution.
Tests: `make check`.
QA: bin/juju- quickstart {bundle}` bin/juju- quickstart mediawiki-single bin/juju- quickstart u/bigdata- dev/apache- analytics- sql bin/juju- quickstart bundle: mediawiki/ scalable bin/juju- quickstart ~landscape/ landscape- dense-maas/ landscape- dense-maas bin/juju- quickstart bundle: django/ example- single
install bundles with quickstart:
`devenv/
Try the following bundles:
- devenv/
- devenv/
- devenv/
- devenv/
bundle:
- devenv/
Those instead should return errors: bin/juju- quickstart mediawiki/trusty bin/juju- quickstart mediawiki-nosuch bin/juju- quickstart no-such bin/juju- quickstart bundle:no/such bin/juju- quickstart bundle:invalid bin/juju- quickstart ~landscape/ landscape- dense-maas/ landscape
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
- devenv/
bundle:
https:/ /code.launchpad .net/~frankban/ juju-quickstart /old-style- bundles- regression/ +merge/ 252353
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/215070043/
Affected files (+856, -266 lines): __init_ _.py charmstore. py manage. py models/ bundles. py models/ references. py netutils. py settings. py tests/helpers. py tests/models/ test_bundles. py tests/models/ test_references .py tests/test_ app.py tests/test_ charmstore. py tests/test_ netutils. py
A [revision details]
M quickstart/
M quickstart/app.py
A quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
A quickstart/
M quickstart/