Merge lp:~frankban/juju-quickstart/unicode-all-the-things into lp:juju-quickstart
- unicode-all-the-things
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 30 |
Proposed branch: | lp:~frankban/juju-quickstart/unicode-all-the-things |
Merge into: | lp:juju-quickstart |
Diff against target: |
1249 lines (+370/-148) 19 files modified
juju-quickstart (+3/-1) quickstart/__init__.py (+4/-1) quickstart/app.py (+13/-5) quickstart/juju.py (+4/-4) quickstart/manage.py (+27/-7) quickstart/models/charms.py (+27/-16) quickstart/models/envs.py (+15/-12) quickstart/serializers.py (+47/-0) quickstart/settings.py (+2/-0) quickstart/tests/helpers.py (+3/-1) quickstart/tests/models/__init__.py (+15/-0) quickstart/tests/models/test_charms.py (+14/-12) quickstart/tests/models/test_envs.py (+3/-1) quickstart/tests/test_app.py (+4/-2) quickstart/tests/test_juju.py (+7/-5) quickstart/tests/test_manage.py (+32/-5) quickstart/tests/test_serializers.py (+58/-0) quickstart/tests/test_utils.py (+46/-38) quickstart/utils.py (+46/-38) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/unicode-all-the-things |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+198112@code.launchpad.net |
Commit message
Description of the change
Use unicode everywhere in the application.
My apologies for this branch, the diff is long
and the contents themself are boring: I am very
sorry. Anyway, I think this pushes quickstart
in a better place re: unicode/bytes handling.
Everything here is almost mechanical, including
the annoying problem of having to store encoded
strings in the exceptions: please let me know
if you have any suggestions, or if I am missing
something.
The serializers module is new stuff: it allows
for loading YAML strings as unicode and for
dumping dictionaries in the extended format
(used e.g. by juju-core in the envs.yaml file).
To QA this, just use quickstart as usual, deploying
a bundle or spinning up an environment.
Then try to break it passing exotic strings as
options, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url ☢
(FWIW the above character is \xe2\x98\xa2).
This obviously fail, but the error should not
be unicode related.
Thank you and sorry again.
Francesco Banconi (frankban) wrote : | # |
Madison Scott-Clary (makyo) wrote : | # |
LGTM, QA okay. Thanks for the work!
Richard Harding (rharding) wrote : | # |
My tears run deep. I didn't realize it would be this messy supporting
both versions. Does it make any sense to create an alternate ValueError
that can be auto typed?
Yay for working on getting the ball rolling. LGTM with notes below.
https:/
File quickstart/
https:/
quickstart/
How does this work with py3 when there is no unicode function? I'd seen
things like
https:/
Which creates a unicode reference to str for py3 support.
https:/
File quickstart/
https:/
quickstart/
charms.
I was expecting these to be u'' strings now.
Francesco Banconi (frankban) wrote : | # |
On 2013/12/06 19:17:34, rharding wrote:
> My tears run deep. I didn't realize it would be this messy supporting
both
> versions. Does it make any sense to create an alternate ValueError
that can be
> auto typed?
To be clear, with this branch we do not support both versions,
but just make the unicode handling more sane and nearer to
Python 3. Given this is not a library, I am not even sure we
will need to support both version: we can eventually just switch
to Python3 and drop support for previous versions.
An alternate ValueError could be a nice idea: on the
other hand I liked the way we reuse already existing exception
when possible, and IMHO the more I think about being explicit
when encoding exception messages, the more it does not seem
so bad to me. When we will make the effort to port everything
to Python3, we can remove all those unnecessary "msg.encode"
bits.
> Yay for working on getting the ball rolling. LGTM with notes below.
> https:/
> File quickstart/
https:/
> quickstart/
> How does this work with py3 when there is no unicode function? I'd
seen things
> like
https:/
> Which creates a unicode reference to str for py3 support.
As mentioned above, we are still not supporting Python3 (including
various __unicode__
and __str__ methods we still have in the code with Python2 semantics).
For this reason we can avoid introducing that kind of conditionals in
the code.
https:/
> File quickstart/
https:/
> quickstart/
charms.
> I was expecting these to be u'' strings now.
Are you referring to:
expected = 'charm URL has no schema: precise/juju-gui'?
If so, it is still not clear if you expected 1) the whole
string to be u'...' or 2) just the final part, e.g.:
"charm URL has no schema: u'precise/
Case 1) we use unicode_literals from __future__.
Case 2) we no longer print the repr of the value in the error,
but just the value itself.
Thank you both for the reviews!
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Use unicode everywhere in the application.
My apologies for this branch, the diff is long
and the contents themself are boring: I am very
sorry. Anyway, I think this pushes quickstart
in a better place re: unicode/bytes handling.
Everything here is almost mechanical, including
the annoying problem of having to store encoded
strings in the exceptions: please let me know
if you have any suggestions, or if I am missing
something.
The serializers module is new stuff: it allows
for loading YAML strings as unicode and for
dumping dictionaries in the extended format
(used e.g. by juju-core in the envs.yaml file).
To QA this, just use quickstart as usual, deploying
a bundle or spinning up an environment.
Then try to break it passing exotic strings as
options, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url ☢
(FWIW the above character is \xe2\x98\xa2).
This obviously fail, but the error should not
be unicode related.
Thank you and sorry again.
R=matthew.scott, rharding
CC=
https:/
Preview Diff
1 | === modified file 'juju-quickstart' |
2 | --- juju-quickstart 2013-10-30 10:16:36 +0000 |
3 | +++ juju-quickstart 2013-12-06 18:31:03 +0000 |
4 | @@ -18,6 +18,8 @@ |
5 | |
6 | """Juju Quickstart plugin entry point.""" |
7 | |
8 | +from __future__ import unicode_literals |
9 | + |
10 | import sys |
11 | |
12 | from quickstart import ( |
13 | @@ -31,4 +33,4 @@ |
14 | try: |
15 | manage.run(options) |
16 | except app.ProgramExit as err: |
17 | - sys.exit(str(err)) |
18 | + sys.exit(bytes(err)) |
19 | |
20 | === modified file 'quickstart/__init__.py' |
21 | --- quickstart/__init__.py 2013-12-05 08:49:12 +0000 |
22 | +++ quickstart/__init__.py 2013-12-06 18:31:03 +0000 |
23 | @@ -19,9 +19,12 @@ |
24 | that it can be managed using a Web interface (the Juju GUI). |
25 | """ |
26 | |
27 | +from __future__ import unicode_literals |
28 | + |
29 | + |
30 | VERSION = (0, 5, 0) |
31 | |
32 | |
33 | def get_version(): |
34 | """Return the Juju Quickstart version as a string.""" |
35 | - return '.'.join(map(str, VERSION)) |
36 | + return '.'.join(map(unicode, VERSION)) |
37 | |
38 | === modified file 'quickstart/app.py' |
39 | --- quickstart/app.py 2013-12-06 16:17:45 +0000 |
40 | +++ quickstart/app.py 2013-12-06 18:31:03 +0000 |
41 | @@ -16,7 +16,11 @@ |
42 | |
43 | """Juju Quickstart base application functions.""" |
44 | |
45 | -from __future__ import print_function |
46 | +from __future__ import ( |
47 | + print_function, |
48 | + unicode_literals, |
49 | +) |
50 | + |
51 | import functools |
52 | import json |
53 | import logging |
54 | @@ -36,13 +40,17 @@ |
55 | |
56 | Raise this exception if you want the program to exit gracefully printing |
57 | the error message to stderr. |
58 | + |
59 | + The error message can be either a unicode or a byte string. |
60 | """ |
61 | |
62 | def __init__(self, message): |
63 | + if isinstance(message, unicode): |
64 | + message = message.encode('utf-8') |
65 | self.message = message |
66 | |
67 | def __str__(self): |
68 | - return 'juju-quickstart: error: {}'.format(self.message) |
69 | + return b'juju-quickstart: error: {}'.format(self.message) |
70 | |
71 | |
72 | def ensure_dependencies(): |
73 | @@ -69,7 +77,7 @@ |
74 | try: |
75 | utils.add_apt_repository('ppa:juju/stable') |
76 | except OSError as err: |
77 | - raise ProgramExit(str(err)) |
78 | + raise ProgramExit(bytes(err)) |
79 | print('installing the following packages: {} ' |
80 | '(this can take a while)'.format(', '.join(required_packages))) |
81 | print('sudo privileges will be used for package installation') |
82 | @@ -220,8 +228,8 @@ |
83 | env = juju.connect(api_url) |
84 | except Exception as err: |
85 | try_count += 1 |
86 | - msg = 'unable to connect to the Juju API server on {}: {}'.format( |
87 | - api_url, err) |
88 | + msg = b'unable to connect to the Juju API server on {}: {}'.format( |
89 | + api_url.encode('utf-8'), err) |
90 | if try_count >= 30: |
91 | raise ProgramExit(msg) |
92 | else: |
93 | |
94 | === modified file 'quickstart/juju.py' |
95 | --- quickstart/juju.py 2013-11-27 15:58:06 +0000 |
96 | +++ quickstart/juju.py 2013-12-06 18:31:03 +0000 |
97 | @@ -16,13 +16,13 @@ |
98 | |
99 | """Juju Quickstart API client.""" |
100 | |
101 | +from __future__ import unicode_literals |
102 | + |
103 | import logging |
104 | |
105 | import jujuclient |
106 | import websocket |
107 | |
108 | -from quickstart import utils |
109 | - |
110 | |
111 | def connect(api_url): |
112 | """Return an Environment instance connected to the given API URL.""" |
113 | @@ -147,7 +147,7 @@ |
114 | |
115 | Overridden to add logging. |
116 | """ |
117 | - logging.debug('API message: --> {}'.format(utils.utf8(message))) |
118 | + logging.debug('API message: --> {}'.format(message.decode('utf-8'))) |
119 | return super(WebSocketConnection, self).send(message) |
120 | |
121 | def recv(self): |
122 | @@ -156,5 +156,5 @@ |
123 | Overridden to add logging. |
124 | """ |
125 | message = super(WebSocketConnection, self).recv() |
126 | - logging.debug('API message: <-- {}'.format(utils.utf8(message))) |
127 | + logging.debug('API message: <-- {}'.format(message.decode('utf-8'))) |
128 | return message |
129 | |
130 | === modified file 'quickstart/manage.py' |
131 | --- quickstart/manage.py 2013-12-05 19:54:20 +0000 |
132 | +++ quickstart/manage.py 2013-12-06 18:31:03 +0000 |
133 | @@ -16,10 +16,16 @@ |
134 | |
135 | """Juju Quickstart application management.""" |
136 | |
137 | -from __future__ import print_function |
138 | +from __future__ import ( |
139 | + print_function, |
140 | + unicode_literals, |
141 | +) |
142 | + |
143 | import argparse |
144 | +import codecs |
145 | import logging |
146 | import os |
147 | +import sys |
148 | import webbrowser |
149 | |
150 | import quickstart |
151 | @@ -76,7 +82,8 @@ |
152 | if os.path.isdir(bundle_file): |
153 | bundle_file = os.path.join(bundle_file, 'bundles.yaml') |
154 | try: |
155 | - bundle_yaml = open(bundle_file).read() |
156 | + bundle_yaml = codecs.open( |
157 | + bundle_file.encode('utf-8'), encoding='utf-8').read() |
158 | except IOError as err: |
159 | return parser.error('unable to open bundle file: {}'.format(err)) |
160 | # Validate the bundle. |
161 | @@ -84,7 +91,7 @@ |
162 | bundle_name, bundle_services = utils.parse_bundle( |
163 | bundle_yaml, options.bundle_name) |
164 | except ValueError as err: |
165 | - return parser.error(str(err)) |
166 | + return parser.error(bytes(err)) |
167 | # Update the options namespace with the new values. |
168 | options.bundle_name = bundle_name |
169 | options.bundle_services = bundle_services |
170 | @@ -107,12 +114,12 @@ |
171 | try: |
172 | charm = charms.Charm.from_url(options.charm_url) |
173 | except ValueError as err: |
174 | - return parser.error(str(err)) |
175 | + return parser.error(bytes(err)) |
176 | if charm.is_local(): |
177 | - return parser.error('local charms are not allowed: {}'.format(charm)) |
178 | + return parser.error(b'local charms are not allowed: {}'.format(charm)) |
179 | if charm.series not in settings.JUJU_GUI_SUPPORTED_SERIES: |
180 | return parser.error( |
181 | - 'unsupported charm series: {!r}'.format(charm.series)) |
182 | + 'unsupported charm series: {}'.format(charm.series)) |
183 | if ( |
184 | # The user requested a bundle deployment. |
185 | options.bundle and |
186 | @@ -150,7 +157,7 @@ |
187 | env_type, admin_secret, default_series = ( |
188 | envs.parse_env_file(env_file, env_name)) |
189 | except ValueError as err: |
190 | - return parser.error(str(err)) |
191 | + return parser.error(bytes(err)) |
192 | # Update the options namespace with the new values. |
193 | options.admin_secret = admin_secret |
194 | options.env_file = env_file |
195 | @@ -175,6 +182,17 @@ |
196 | ) |
197 | |
198 | |
199 | +def _convert_options_to_unicode(options): |
200 | + """Convert all byte string values in the options namespace to unicode. |
201 | + |
202 | + Modify the options in place and return None. |
203 | + """ |
204 | + encoding = sys.stdin.encoding or 'utf-8' |
205 | + for key, value in options._get_kwargs(): |
206 | + if isinstance(value, bytes): |
207 | + setattr(options, key, value.decode(encoding)) |
208 | + |
209 | + |
210 | def setup(): |
211 | """Set up the application options and logger. |
212 | |
213 | @@ -253,6 +271,8 @@ |
214 | nargs=0, help="Show program's description and exit") |
215 | # Parse the provided arguments. |
216 | options = parser.parse_args() |
217 | + # Convert the provided string arguments to unicode. |
218 | + _convert_options_to_unicode(options) |
219 | # Validate and process the provided arguments. |
220 | _validate_env(options, parser) |
221 | if options.bundle is not None: |
222 | |
223 | === modified file 'quickstart/models/charms.py' |
224 | --- quickstart/models/charms.py 2013-12-03 22:48:26 +0000 |
225 | +++ quickstart/models/charms.py 2013-12-06 18:31:03 +0000 |
226 | @@ -16,6 +16,8 @@ |
227 | |
228 | """Juju Quickstart charms management.""" |
229 | |
230 | +from __future__ import unicode_literals |
231 | + |
232 | import re |
233 | |
234 | |
235 | @@ -39,45 +41,50 @@ |
236 | try: |
237 | schema, remaining = url.split(':', 1) |
238 | except ValueError: |
239 | - raise ValueError('charm URL has no schema: {!r}'.format(url)) |
240 | + msg = 'charm URL has no schema: {}'.format(url) |
241 | + raise ValueError(msg.encode('utf-8')) |
242 | if schema not in ('cs', 'local'): |
243 | - raise ValueError('charm URL has invalid schema: {!r}'.format(schema)) |
244 | + msg = 'charm URL has invalid schema: {}'.format(schema) |
245 | + raise ValueError(msg.encode('utf-8')) |
246 | # Retrieve the optional user, the series, name and revision. |
247 | parts = remaining.split('/') |
248 | parts_length = len(parts) |
249 | if parts_length == 3: |
250 | user, series, name_revision = parts |
251 | if not user.startswith('~'): |
252 | - raise ValueError( |
253 | - 'charm URL has invalid user name form: {!r}'.format(user)) |
254 | + msg = 'charm URL has invalid user name form: {}'.format(user) |
255 | + raise ValueError(msg.encode('utf-8')) |
256 | user = user[1:] |
257 | if not valid_user(user): |
258 | - raise ValueError( |
259 | - 'charm URL has invalid user name: {!r}'.format(user)) |
260 | + msg = 'charm URL has invalid user name: {}'.format(user) |
261 | + raise ValueError(msg.encode('utf-8')) |
262 | if schema == 'local': |
263 | - raise ValueError( |
264 | - 'local charm URL with user name: {!r}'.format(url)) |
265 | + msg = 'local charm URL with user name: {}'.format(url) |
266 | + raise ValueError(msg.encode('utf-8')) |
267 | elif parts_length == 2: |
268 | user = '' |
269 | series, name_revision = parts |
270 | else: |
271 | - raise ValueError('charm URL has invalid form: {!r}'.format(url)) |
272 | + msg = 'charm URL has invalid form: {}'.format(url) |
273 | + raise ValueError(msg.encode('utf-8')) |
274 | # Validate the series. |
275 | if not valid_series(series): |
276 | - raise ValueError('charm URL has invalid series: {!r}'.format(series)) |
277 | + msg = 'charm URL has invalid series: {}'.format(series) |
278 | + raise ValueError(msg.encode('utf-8')) |
279 | # Validate name and revision. |
280 | try: |
281 | name, revision = name_revision.rsplit('-', 1) |
282 | except ValueError: |
283 | - raise ValueError( |
284 | - 'charm URL has no revision: {!r}'.format(url)) |
285 | + msg = 'charm URL has no revision: {}'.format(url) |
286 | + raise ValueError(msg.encode('utf-8')) |
287 | if not valid_name(name): |
288 | - raise ValueError('charm URL has invalid name: {!r}'.format(name)) |
289 | + msg = 'charm URL has invalid name: {}'.format(name) |
290 | + raise ValueError(msg.encode('utf-8')) |
291 | try: |
292 | revision = int(revision) |
293 | except ValueError: |
294 | - raise ValueError( |
295 | - 'charm URL has invalid revision: {!r}'.format(revision)) |
296 | + msg = 'charm URL has invalid revision: {}'.format(revision) |
297 | + raise ValueError(msg.encode('utf-8')) |
298 | return schema, user, series, name, revision |
299 | |
300 | |
301 | @@ -102,10 +109,14 @@ |
302 | |
303 | def __str__(self): |
304 | """The string representation of a charm is its URL.""" |
305 | + return self.__unicode__().encode('utf-8') |
306 | + |
307 | + def __unicode__(self): |
308 | + """The unicode representation of a charm is its URL.""" |
309 | return self.url() |
310 | |
311 | def __repr__(self): |
312 | - return '<Charm: {}>'.format(str(self)) |
313 | + return b'<Charm: {}>'.format(bytes(self)) |
314 | |
315 | def url(self): |
316 | """Return the charm URL.""" |
317 | |
318 | === modified file 'quickstart/models/envs.py' |
319 | --- quickstart/models/envs.py 2013-12-04 18:46:32 +0000 |
320 | +++ quickstart/models/envs.py 2013-12-06 18:31:03 +0000 |
321 | @@ -16,12 +16,15 @@ |
322 | |
323 | """Juju Quickstart environments management.""" |
324 | |
325 | +from __future__ import unicode_literals |
326 | + |
327 | import os |
328 | import re |
329 | |
330 | -import yaml |
331 | - |
332 | -from quickstart import utils |
333 | +from quickstart import ( |
334 | + serializers, |
335 | + utils |
336 | +) |
337 | |
338 | |
339 | # Compile the regular expression used to parse the "juju switch" output. |
340 | @@ -75,34 +78,34 @@ |
341 | """ |
342 | # Load the Juju environments file. |
343 | try: |
344 | - environments_file = open(env_file) |
345 | + environments_file = open(env_file.encode('utf-8')) |
346 | except IOError as err: |
347 | - msg = 'unable to open environments file: {}'.format(err) |
348 | + msg = b'unable to open environments file: {}'.format(err) |
349 | raise ValueError(msg) |
350 | # Parse the Juju environments file. |
351 | try: |
352 | - environments = yaml.safe_load(environments_file) |
353 | + environments = serializers.yaml_load(environments_file) |
354 | except Exception as err: |
355 | - msg = 'unable to parse environments file {}: {}'.format(env_file, err) |
356 | - raise ValueError(msg) |
357 | + msg = b'unable to parse environments file {}: {}' |
358 | + raise ValueError(msg.format(env_file.encode('utf-8'), err)) |
359 | # Retrieve the information about the current environment. |
360 | try: |
361 | environment = environments.get('environments', {}).get(env_name) |
362 | except AttributeError as err: |
363 | msg = 'invalid YAML contents in {}: {}'.format(env_file, environments) |
364 | - raise ValueError(msg) |
365 | + raise ValueError(msg.encode('utf-8')) |
366 | if environment is None: |
367 | msg = 'environment {} not found in {}'.format(env_name, env_file) |
368 | - raise ValueError(msg) |
369 | + raise ValueError(msg.encode('utf-8')) |
370 | # Retrieve the provider type and the admin secret. |
371 | env_type = environment.get('type') |
372 | if env_type is None: |
373 | msg = '{} provider type not found in {}'.format(env_name, env_file) |
374 | - raise ValueError(msg) |
375 | + raise ValueError(msg.encode('utf-8')) |
376 | admin_secret = environment.get('admin-secret') |
377 | if admin_secret is None: |
378 | msg = '{} admin secret not found in {}'.format(env_name, env_file) |
379 | - raise ValueError(msg) |
380 | + raise ValueError(msg.encode('utf-8')) |
381 | # The default-series is not required to be set so None is ok. |
382 | default_series = environment.get('default-series') |
383 | return env_type, admin_secret, default_series |
384 | |
385 | === added file 'quickstart/serializers.py' |
386 | --- quickstart/serializers.py 1970-01-01 00:00:00 +0000 |
387 | +++ quickstart/serializers.py 2013-12-06 18:31:03 +0000 |
388 | @@ -0,0 +1,47 @@ |
389 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
390 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
391 | +# Copyright (C) 2013 Canonical Ltd. |
392 | +# |
393 | +# This program is free software: you can redistribute it and/or modify it under |
394 | +# the terms of the GNU Affero General Public License version 3, as published by |
395 | +# the Free Software Foundation. |
396 | +# |
397 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
398 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
399 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
400 | +# Affero General Public License for more details. |
401 | +# |
402 | +# You should have received a copy of the GNU Affero General Public License |
403 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
404 | + |
405 | +"""Juju Quickstart functions for serializing data structures.""" |
406 | + |
407 | +from __future__ import unicode_literals |
408 | + |
409 | +import yaml |
410 | + |
411 | + |
412 | +def yaml_load(stream): |
413 | + """Parse the YAML document in stream. |
414 | + |
415 | + The stream argument can be a file like object or a string content. |
416 | + |
417 | + Produce and return the corresponding Python object, returning strings |
418 | + as unicode objects. |
419 | + """ |
420 | + # See <http://stackoverflow.com/questions/2890146/ |
421 | + # how-to-force-pyyaml-to-load-strings-as-unicode-objects>. |
422 | + loader_class = type(b'UnicodeLoader', (yaml.SafeLoader,), {}) |
423 | + loader_class.add_constructor( |
424 | + 'tag:yaml.org,2002:str', |
425 | + lambda loader, node: loader.construct_scalar(node)) |
426 | + return yaml.load(stream, Loader=loader_class) |
427 | + |
428 | + |
429 | +def yaml_dump(data, stream=None): |
430 | + """Serialize a Python object into a YAML stream. |
431 | + |
432 | + If stream is None, return the produced string instead. |
433 | + Always serialize collections in the block style. |
434 | + """ |
435 | + return yaml.safe_dump(data, stream, default_flow_style=False) |
436 | |
437 | === modified file 'quickstart/settings.py' |
438 | --- quickstart/settings.py 2013-12-04 14:47:58 +0000 |
439 | +++ quickstart/settings.py 2013-12-06 18:31:03 +0000 |
440 | @@ -16,6 +16,8 @@ |
441 | |
442 | """Juju Quickstart settings.""" |
443 | |
444 | +from __future__ import unicode_literals |
445 | + |
446 | import os |
447 | |
448 | |
449 | |
450 | === modified file 'quickstart/tests/helpers.py' |
451 | --- quickstart/tests/helpers.py 2013-12-04 14:47:58 +0000 |
452 | +++ quickstart/tests/helpers.py 2013-12-06 18:31:03 +0000 |
453 | @@ -16,6 +16,8 @@ |
454 | |
455 | """Test helpers for the Juju Quickstart plugin.""" |
456 | |
457 | +from __future__ import unicode_literals |
458 | + |
459 | from contextlib import contextmanager |
460 | import os |
461 | import shutil |
462 | @@ -156,7 +158,7 @@ |
463 | """ |
464 | with self.assertRaises(ValueError) as context_manager: |
465 | yield |
466 | - self.assertEqual(error, str(context_manager.exception)) |
467 | + self.assertEqual(error, bytes(context_manager.exception)) |
468 | |
469 | |
470 | class WatcherDataTestsMixin(object): |
471 | |
472 | === modified file 'quickstart/tests/models/__init__.py' |
473 | --- quickstart/tests/models/__init__.py 2013-12-03 22:48:26 +0000 |
474 | +++ quickstart/tests/models/__init__.py 2013-12-06 18:31:03 +0000 |
475 | @@ -0,0 +1,15 @@ |
476 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
477 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
478 | +# Copyright (C) 2013 Canonical Ltd. |
479 | +# |
480 | +# This program is free software: you can redistribute it and/or modify it under |
481 | +# the terms of the GNU Affero General Public License version 3, as published by |
482 | +# the Free Software Foundation. |
483 | +# |
484 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
485 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
486 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
487 | +# Affero General Public License for more details. |
488 | +# |
489 | +# You should have received a copy of the GNU Affero General Public License |
490 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
491 | |
492 | === modified file 'quickstart/tests/models/test_charms.py' |
493 | --- quickstart/tests/models/test_charms.py 2013-12-03 22:48:26 +0000 |
494 | +++ quickstart/tests/models/test_charms.py 2013-12-06 18:31:03 +0000 |
495 | @@ -16,6 +16,8 @@ |
496 | |
497 | """Tests for the Juju Quickstart charms management.""" |
498 | |
499 | +from __future__ import unicode_literals |
500 | + |
501 | import unittest |
502 | |
503 | from quickstart.models import charms |
504 | @@ -26,25 +28,25 @@ |
505 | |
506 | def test_no_schema_error(self): |
507 | # A ValueError is raised if the URL schema is missing. |
508 | - expected = "charm URL has no schema: 'precise/juju-gui'" |
509 | + expected = 'charm URL has no schema: precise/juju-gui' |
510 | with self.assert_value_error(expected): |
511 | charms.parse_url('precise/juju-gui') |
512 | |
513 | def test_invalid_schema_error(self): |
514 | # A ValueError is raised if the URL schema is not valid. |
515 | - expected = "charm URL has invalid schema: 'http'" |
516 | + expected = 'charm URL has invalid schema: http' |
517 | with self.assert_value_error(expected): |
518 | charms.parse_url('http:precise/juju-gui') |
519 | |
520 | def test_invalid_user_form_error(self): |
521 | # A ValueError is raised if the user form is not valid. |
522 | - expected = "charm URL has invalid user name form: 'jean-luc'" |
523 | + expected = 'charm URL has invalid user name form: jean-luc' |
524 | with self.assert_value_error(expected): |
525 | charms.parse_url('cs:jean-luc/precise/juju-gui') |
526 | |
527 | def test_invalid_user_name_error(self): |
528 | # A ValueError is raised if the user name is not valid. |
529 | - expected = "charm URL has invalid user name: 'jean:luc'" |
530 | + expected = 'charm URL has invalid user name: jean:luc' |
531 | with self.assert_value_error(expected): |
532 | charms.parse_url('cs:~jean:luc/precise/juju-gui') |
533 | |
534 | @@ -52,37 +54,37 @@ |
535 | # A ValueError is raised if a user is specified on a local charm. |
536 | expected = ( |
537 | 'local charm URL with user name: ' |
538 | - "'local:~jean-luc/precise/juju-gui'") |
539 | + 'local:~jean-luc/precise/juju-gui') |
540 | with self.assert_value_error(expected): |
541 | charms.parse_url('local:~jean-luc/precise/juju-gui') |
542 | |
543 | def test_invalid_form_error(self): |
544 | # A ValueError is raised if the URL is not valid. |
545 | - expected = "charm URL has invalid form: 'cs:~user/series/name/what-?'" |
546 | + expected = 'charm URL has invalid form: cs:~user/series/name/what-?' |
547 | with self.assert_value_error(expected): |
548 | charms.parse_url('cs:~user/series/name/what-?') |
549 | |
550 | def test_invalid_series_error(self): |
551 | # A ValueError is raised if the series is not valid. |
552 | - expected = "charm URL has invalid series: 'boo!'" |
553 | + expected = 'charm URL has invalid series: boo!' |
554 | with self.assert_value_error(expected): |
555 | charms.parse_url('cs:boo!/juju-gui-42') |
556 | |
557 | def test_no_revision_error(self): |
558 | # A ValueError is raised if the charm revision is missing. |
559 | - expected = "charm URL has no revision: 'cs:series/name'" |
560 | + expected = 'charm URL has no revision: cs:series/name' |
561 | with self.assert_value_error(expected): |
562 | charms.parse_url('cs:series/name') |
563 | |
564 | def test_invalid_revision_error(self): |
565 | # A ValueError is raised if the charm revision is not valid. |
566 | - expected = "charm URL has invalid revision: 'revision'" |
567 | + expected = 'charm URL has invalid revision: revision' |
568 | with self.assert_value_error(expected): |
569 | charms.parse_url('cs:series/name-revision') |
570 | |
571 | def test_invalid_name_error(self): |
572 | # A ValueError is raised if the charm name is not valid. |
573 | - expected = "charm URL has invalid name: 'not:valid'" |
574 | + expected = 'charm URL has invalid name: not:valid' |
575 | with self.assert_value_error(expected): |
576 | charms.parse_url('cs:precise/not:valid-42') |
577 | |
578 | @@ -169,14 +171,14 @@ |
579 | def test_from_url_error(self): |
580 | # A ValueError is raised by the from_url class method if the provided |
581 | # URL is not a valid charm URL. |
582 | - expected = "charm URL has invalid form: 'cs:not-a-charm-url'" |
583 | + expected = 'charm URL has invalid form: cs:not-a-charm-url' |
584 | with self.assert_value_error(expected): |
585 | charms.Charm.from_url('cs:not-a-charm-url') |
586 | |
587 | def test_string(self): |
588 | # The string representation of a charm instance is its URL. |
589 | charm = self.make_charm() |
590 | - self.assertEqual('cs:~myuser/precise/juju-gui-42', str(charm)) |
591 | + self.assertEqual('cs:~myuser/precise/juju-gui-42', bytes(charm)) |
592 | |
593 | def test_repr(self): |
594 | # A charm instance is correctly represented. |
595 | |
596 | === modified file 'quickstart/tests/models/test_envs.py' |
597 | --- quickstart/tests/models/test_envs.py 2013-12-04 18:55:28 +0000 |
598 | +++ quickstart/tests/models/test_envs.py 2013-12-06 18:31:03 +0000 |
599 | @@ -16,6 +16,8 @@ |
600 | |
601 | """Tests for the Juju Quickstart environments management.""" |
602 | |
603 | +from __future__ import unicode_literals |
604 | + |
605 | import unittest |
606 | |
607 | import mock |
608 | @@ -93,7 +95,7 @@ |
609 | with self.assertRaises(ValueError) as context_manager: |
610 | envs.parse_env_file(env_file, 'ec2') |
611 | expected = 'unable to parse environments file {}'.format(env_file) |
612 | - self.assertIn(expected, str(context_manager.exception)) |
613 | + self.assertIn(expected, bytes(context_manager.exception)) |
614 | |
615 | def test_invalid_yaml_contents(self): |
616 | # A ValueError is raised if the environments file is not well formed. |
617 | |
618 | === modified file 'quickstart/tests/test_app.py' |
619 | --- quickstart/tests/test_app.py 2013-12-06 16:17:45 +0000 |
620 | +++ quickstart/tests/test_app.py 2013-12-06 18:31:03 +0000 |
621 | @@ -16,6 +16,8 @@ |
622 | |
623 | """Tests for the Juju Quickstart base application functions.""" |
624 | |
625 | +from __future__ import unicode_literals |
626 | + |
627 | from contextlib import contextmanager |
628 | import json |
629 | import unittest |
630 | @@ -36,7 +38,7 @@ |
631 | def test_string_representation(self): |
632 | # The error is properly represented as a string. |
633 | exception = app.ProgramExit('bad wolf') |
634 | - self.assertEqual('juju-quickstart: error: bad wolf', str(exception)) |
635 | + self.assertEqual('juju-quickstart: error: bad wolf', bytes(exception)) |
636 | |
637 | |
638 | class ProgramExitTestsMixin(object): |
639 | @@ -51,7 +53,7 @@ |
640 | with self.assertRaises(app.ProgramExit) as context_manager: |
641 | yield |
642 | expected = 'juju-quickstart: error: {}'.format(error) |
643 | - self.assertEqual(expected, str(context_manager.exception)) |
644 | + self.assertEqual(expected, bytes(context_manager.exception)) |
645 | |
646 | def make_env_error(self, message): |
647 | """Create and return a jujuclient.EnvError with the given message.""" |
648 | |
649 | === modified file 'quickstart/tests/test_juju.py' |
650 | --- quickstart/tests/test_juju.py 2013-11-27 15:58:06 +0000 |
651 | +++ quickstart/tests/test_juju.py 2013-12-06 18:31:03 +0000 |
652 | @@ -16,6 +16,8 @@ |
653 | |
654 | """Tests for the Juju Quickstart API client.""" |
655 | |
656 | +from __future__ import unicode_literals |
657 | + |
658 | import unittest |
659 | |
660 | import mock |
661 | @@ -317,7 +319,7 @@ |
662 | |
663 | class TestWebSocketConnection(unittest.TestCase): |
664 | |
665 | - snowman = u'Here is a snowman\u00a1: \u2603' |
666 | + snowman = 'Here is a snowman\u00a1: \u2603' |
667 | |
668 | def setUp(self): |
669 | with mock.patch('socket.socket') as mock_socket: |
670 | @@ -335,9 +337,9 @@ |
671 | |
672 | def test_send_unicode(self): |
673 | # Outgoing unicode messages are properly logged. |
674 | - expected = 'API message: --> {}'.format(self.snowman.encode('utf-8')) |
675 | + expected = 'API message: --> {}'.format(self.snowman) |
676 | with helpers.assert_logs([expected], 'debug'): |
677 | - self.conn.send(self.snowman) |
678 | + self.conn.send(self.snowman.encode('utf-8')) |
679 | self.assertTrue(self.mock_send.called) |
680 | |
681 | def test_recv(self): |
682 | @@ -349,8 +351,8 @@ |
683 | |
684 | def test_recv_unicode(self): |
685 | # Incoming unicode messages are properly logged. |
686 | - self.mock_recv.return_value = (42, self.snowman) |
687 | - expected = 'API message: <-- {}'.format(self.snowman.encode('utf-8')) |
688 | + self.mock_recv.return_value = (42, self.snowman.encode('utf-8')) |
689 | + expected = 'API message: <-- {}'.format(self.snowman) |
690 | with helpers.assert_logs([expected], 'debug'): |
691 | self.conn.recv() |
692 | self.mock_recv.assert_called_once_with() |
693 | |
694 | === modified file 'quickstart/tests/test_manage.py' |
695 | --- quickstart/tests/test_manage.py 2013-12-05 19:54:20 +0000 |
696 | +++ quickstart/tests/test_manage.py 2013-12-06 18:31:03 +0000 |
697 | @@ -16,10 +16,13 @@ |
698 | |
699 | """Tests for the Juju Quickstart management infrastructure.""" |
700 | |
701 | +from __future__ import unicode_literals |
702 | + |
703 | import argparse |
704 | import logging |
705 | import os |
706 | import shutil |
707 | +import StringIO as io |
708 | import tempfile |
709 | import unittest |
710 | |
711 | @@ -166,7 +169,7 @@ |
712 | options = self.make_options(url) |
713 | manage._validate_bundle(options, self.parser) |
714 | self.parser.error.assert_called_once_with( |
715 | - "unable to open the bundle: invalid bundle URL: 'bundle:'") |
716 | + 'unable to open the bundle: invalid bundle URL: bundle:') |
717 | |
718 | def test_error_parsing_bundle_contents(self): |
719 | # A parser error is invoked if an error occurs parsing the bundle YAML. |
720 | @@ -194,7 +197,7 @@ |
721 | # A parser error is invoked if the charm URL is not valid. |
722 | options = self.make_options('cs:invalid') |
723 | manage._validate_charm_url(options, self.parser) |
724 | - expected = "charm URL has invalid form: 'cs:invalid'" |
725 | + expected = 'charm URL has invalid form: cs:invalid' |
726 | self.parser.error.assert_called_once_with(expected) |
727 | |
728 | def test_local_charm_error(self): |
729 | @@ -208,7 +211,7 @@ |
730 | # A parser error is invoked if the charm series is not supported. |
731 | options = self.make_options('cs:nosuch/juju-gui-100') |
732 | manage._validate_charm_url(options, self.parser) |
733 | - expected = "unsupported charm series: 'nosuch'" |
734 | + expected = 'unsupported charm series: nosuch' |
735 | self.parser.error.assert_called_once_with(expected) |
736 | |
737 | def test_outdated_charm_error(self): |
738 | @@ -307,6 +310,30 @@ |
739 | self.assertEqual('local', options.env_type) |
740 | |
741 | |
742 | +class TestConvertOptionsToUnicode(unittest.TestCase): |
743 | + |
744 | + def test_bytes_options(self): |
745 | + # Byte strings are correctly converted. |
746 | + options = argparse.Namespace(opt1=b'val1', opt2=b'val2') |
747 | + manage._convert_options_to_unicode(options) |
748 | + self.assertEqual('val1', options.opt1) |
749 | + self.assertIsInstance(options.opt1, unicode) |
750 | + self.assertEqual('val2', options.opt2) |
751 | + self.assertIsInstance(options.opt2, unicode) |
752 | + |
753 | + def test_unicode_options(self): |
754 | + # Unicode options are left untouched. |
755 | + options = argparse.Namespace(myopt='myval') |
756 | + self.assertEqual('myval', options.myopt) |
757 | + self.assertIsInstance(options.myopt, unicode) |
758 | + |
759 | + def test_other_types(self): |
760 | + # Other non-string types are left untouched. |
761 | + options = argparse.Namespace(opt1=42, opt2=None) |
762 | + self.assertEqual(42, options.opt1) |
763 | + self.assertIsNone(options.opt2) |
764 | + |
765 | + |
766 | @mock.patch('quickstart.manage._validate_env', mock.Mock()) |
767 | class TestSetup(unittest.TestCase): |
768 | |
769 | @@ -376,10 +403,10 @@ |
770 | |
771 | def test_version(self): |
772 | # The program version is properly printed to stderr. |
773 | - with mock.patch('sys.stderr') as mock_stderr: |
774 | + with mock.patch('sys.stderr', new_callable=io.StringIO) as mock_stderr: |
775 | self.call_setup(['--version']) |
776 | expected = 'juju-quickstart {}\n'.format(quickstart.get_version()) |
777 | - mock_stderr.write.assert_called_once_with(expected) |
778 | + self.assertEqual(expected, mock_stderr.getvalue()) |
779 | |
780 | @mock.patch('quickstart.manage._validate_bundle') |
781 | def test_bundle(self, mock_validate_bundle): |
782 | |
783 | === added file 'quickstart/tests/test_serializers.py' |
784 | --- quickstart/tests/test_serializers.py 1970-01-01 00:00:00 +0000 |
785 | +++ quickstart/tests/test_serializers.py 2013-12-06 18:31:03 +0000 |
786 | @@ -0,0 +1,58 @@ |
787 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
788 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
789 | +# Copyright (C) 2013 Canonical Ltd. |
790 | +# |
791 | +# This program is free software: you can redistribute it and/or modify it under |
792 | +# the terms of the GNU Affero General Public License version 3, as published by |
793 | +# the Free Software Foundation. |
794 | +# |
795 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
796 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
797 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
798 | +# Affero General Public License for more details. |
799 | +# |
800 | +# You should have received a copy of the GNU Affero General Public License |
801 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
802 | + |
803 | +"""Tests for the Juju Quickstart serialization helpers.""" |
804 | + |
805 | +from __future__ import unicode_literals |
806 | + |
807 | +import unittest |
808 | + |
809 | +import mock |
810 | +import yaml |
811 | + |
812 | +from quickstart import serializers |
813 | + |
814 | + |
815 | +class TestYamlLoad(unittest.TestCase): |
816 | + |
817 | + contents = '{myint: 42, mystring: foo}' |
818 | + |
819 | + def test_unicode_strings(self): |
820 | + # Strings are returned as unicode objects. |
821 | + decoded = serializers.yaml_load(self.contents) |
822 | + self.assertEqual(42, decoded['myint']) |
823 | + self.assertEqual('foo', decoded['mystring']) |
824 | + self.assertIsInstance(decoded['mystring'], unicode) |
825 | + for key in decoded: |
826 | + self.assertIsInstance(key, unicode, key) |
827 | + |
828 | + @mock.patch('quickstart.serializers.yaml.load') |
829 | + def test_safe(self, mock_load): |
830 | + # The YAML decoder uses a safe loader. |
831 | + serializers.yaml_load(self.contents) |
832 | + self.assertEqual(self.contents, mock_load.call_args[0][0]) |
833 | + loader_class = mock_load.call_args[1]['Loader'] |
834 | + self.assertTrue(issubclass(loader_class, yaml.SafeLoader)) |
835 | + |
836 | + |
837 | +class TestYamlDump(unittest.TestCase): |
838 | + |
839 | + data = {'myint': 42, 'mystring': 'foo'} |
840 | + |
841 | + def test_block_style(self): |
842 | + # Collections are serialized in the block style. |
843 | + contents = serializers.yaml_dump(self.data) |
844 | + self.assertEqual('myint: 42\nmystring: foo\n', contents) |
845 | |
846 | === modified file 'quickstart/tests/test_utils.py' |
847 | --- quickstart/tests/test_utils.py 2013-12-04 22:16:45 +0000 |
848 | +++ quickstart/tests/test_utils.py 2013-12-06 18:31:03 +0000 |
849 | @@ -16,6 +16,8 @@ |
850 | |
851 | """Tests for the Juju Quickstart utility functions and classes.""" |
852 | |
853 | +from __future__ import unicode_literals |
854 | + |
855 | import httplib |
856 | import json |
857 | import socket |
858 | @@ -101,7 +103,7 @@ |
859 | 'sudo', self.apt_get, 'install', '-y', |
860 | 'software-properties-common') |
861 | self.assertEqual( |
862 | - 'apt-get install error', str(context_manager.exception)) |
863 | + 'apt-get install error', bytes(context_manager.exception)) |
864 | |
865 | |
866 | class TestCall(unittest.TestCase): |
867 | @@ -202,7 +204,7 @@ |
868 | |
869 | def test_error(self): |
870 | # A ValueError is raised if the given bundle URL is not valid. |
871 | - with self.assert_value_error("invalid bundle URL: 'bundle:'"): |
872 | + with self.assert_value_error('invalid bundle URL: bundle:'): |
873 | utils.convert_bundle_url('bundle:') |
874 | |
875 | |
876 | @@ -222,7 +224,7 @@ |
877 | with self.assertRaises(IOError) as context_manager: |
878 | utils.get_charm_url() |
879 | mock_urlread.assert_called_once_with(settings.CHARMWORLD_API) |
880 | - self.assertEqual('bad wolf', str(context_manager.exception)) |
881 | + self.assertEqual('bad wolf', bytes(context_manager.exception)) |
882 | |
883 | def test_value_error(self): |
884 | # A ValueError is raised if the API response is not valid. |
885 | @@ -232,7 +234,7 @@ |
886 | utils.get_charm_url() |
887 | mock_urlread.assert_called_once_with(settings.CHARMWORLD_API) |
888 | self.assertEqual( |
889 | - 'unable to find the charm URL', str(context_manager.exception)) |
890 | + 'unable to find the charm URL', bytes(context_manager.exception)) |
891 | |
892 | |
893 | class TestGetServiceInfo(helpers.WatcherDataTestsMixin, unittest.TestCase): |
894 | @@ -322,7 +324,7 @@ |
895 | with self.patch_call(1, error='bad wolf') as mock_call: |
896 | with self.assertRaises(OSError) as context_manager: |
897 | utils.get_ubuntu_codename() |
898 | - self.assertEqual('bad wolf', str(context_manager.exception)) |
899 | + self.assertEqual('bad wolf', bytes(context_manager.exception)) |
900 | mock_call.assert_called_once_with('lsb_release', '-cs') |
901 | |
902 | |
903 | @@ -343,38 +345,38 @@ |
904 | with self.assertRaises(ValueError) as context_manager: |
905 | utils.parse_bundle(':') |
906 | expected = 'unable to parse the bundle' |
907 | - self.assertIn(expected, str(context_manager.exception)) |
908 | + self.assertIn(expected, bytes(context_manager.exception)) |
909 | |
910 | def test_yaml_invalid_type(self): |
911 | # A ValueError is raised if the bundle contents are not well formed. |
912 | - with self.assert_value_error("invalid YAML contents: 'a-string'"): |
913 | + with self.assert_value_error('invalid YAML contents: a-string'): |
914 | utils.parse_bundle('a-string') |
915 | |
916 | def test_yaml_invalid_bundle_data(self): |
917 | # A ValueError is raised if bundles are not well formed. |
918 | contents = yaml.safe_dump({'mybundle': 'not valid'}) |
919 | - expected = "invalid YAML contents: {'mybundle': 'not valid'}" |
920 | + expected = 'invalid YAML contents: {mybundle: not valid}\n' |
921 | with self.assert_value_error(expected): |
922 | utils.parse_bundle(contents) |
923 | |
924 | def test_yaml_no_service(self): |
925 | # A ValueError is raised if bundles do not include services. |
926 | contents = yaml.safe_dump({'mybundle': {}}) |
927 | - expected = "invalid YAML contents: {'mybundle': {}}" |
928 | + expected = 'invalid YAML contents: mybundle: {}\n' |
929 | with self.assert_value_error(expected): |
930 | utils.parse_bundle(contents) |
931 | |
932 | def test_yaml_none_bundle_services(self): |
933 | # A ValueError is raised if services are None. |
934 | contents = yaml.safe_dump({'mybundle': {'services': None}}) |
935 | - expected = "invalid YAML contents: {'mybundle': {'services': None}}" |
936 | + expected = 'invalid YAML contents: mybundle: {services: null}\n' |
937 | with self.assert_value_error(expected): |
938 | utils.parse_bundle(contents) |
939 | |
940 | def test_yaml_invalid_bundle_services_type(self): |
941 | # A ValueError is raised if services have an invalid type. |
942 | contents = yaml.safe_dump({'mybundle': {'services': 42}}) |
943 | - expected = "invalid YAML contents: {'mybundle': {'services': 42}}" |
944 | + expected = 'invalid YAML contents: mybundle: {services: 42}\n' |
945 | with self.assert_value_error(expected): |
946 | utils.parse_bundle(contents) |
947 | |
948 | @@ -445,7 +447,7 @@ |
949 | with self.assertRaises(ValueError) as context_manager: |
950 | utils.parse_status_output(':') |
951 | expected = 'unable to parse the output' |
952 | - self.assertIn(expected, str(context_manager.exception)) |
953 | + self.assertIn(expected, bytes(context_manager.exception)) |
954 | |
955 | def test_invalid_yaml_contents(self): |
956 | # A ValueError is raised if the output is not well formed. |
957 | @@ -459,7 +461,7 @@ |
958 | '0': {'agent-version': '1.17.0.1'}, |
959 | }, |
960 | } |
961 | - expected = 'machines:0:agent-state not found in {}'.format(str(data)) |
962 | + expected = 'machines:0:agent-state not found in {}'.format(bytes(data)) |
963 | with self.assert_value_error(expected): |
964 | utils.get_agent_state(yaml.safe_dump(data)) |
965 | |
966 | @@ -480,7 +482,7 @@ |
967 | '0': {'agent-version': '1.17.0.1'}, |
968 | }, |
969 | } |
970 | - expected = 'machines:0:series not found in {}'.format(str(data)) |
971 | + expected = 'machines:0:series not found in {}'.format(bytes(data)) |
972 | with self.assert_value_error(expected): |
973 | utils.get_bootstrap_node_series(yaml.safe_dump(data)) |
974 | |
975 | @@ -547,16 +549,19 @@ |
976 | |
977 | class TestUrlread(unittest.TestCase): |
978 | |
979 | - def patch_urlopen(self, contents=None, error=None): |
980 | + def patch_urlopen(self, contents=None, error=None, content_type=None): |
981 | """Patch the urllib2.urlopen function. |
982 | |
983 | If contents is not None, the read() method of the returned mock object |
984 | returns the given contents. |
985 | + If content_type is provided, the response includes the content type. |
986 | If an error is provided, the call raises the error. |
987 | """ |
988 | - mock_urlopen = mock.Mock() |
989 | + mock_urlopen = mock.MagicMock() |
990 | if contents is not None: |
991 | mock_urlopen().read.return_value = contents |
992 | + if content_type is not None: |
993 | + mock_urlopen().headers = {'content-type': content_type} |
994 | if error is not None: |
995 | mock_urlopen.side_effect = error |
996 | mock_urlopen.reset_mock() |
997 | @@ -564,9 +569,32 @@ |
998 | |
999 | def test_contents(self): |
1000 | # The URL contents are correctly returned. |
1001 | - with self.patch_urlopen(contents='URL contents') as mock_urlopen: |
1002 | + with self.patch_urlopen(contents=b'URL contents') as mock_urlopen: |
1003 | contents = utils.urlread('http://example.com/path/') |
1004 | self.assertEqual('URL contents', contents) |
1005 | + self.assertIsInstance(contents, unicode) |
1006 | + mock_urlopen.assert_called_once_with('http://example.com/path/') |
1007 | + |
1008 | + def test_content_type(self): |
1009 | + # The URL contents are decoded using the site charset. |
1010 | + patch_urlopen = self.patch_urlopen( |
1011 | + contents=b'URL contents: \xf8', # This is not a UTF-8 byte string. |
1012 | + content_type='text/html; charset=ISO-8859-1') |
1013 | + with patch_urlopen as mock_urlopen: |
1014 | + contents = utils.urlread('http://example.com/path/') |
1015 | + self.assertEqual('URL contents: \xf8', contents) |
1016 | + self.assertIsInstance(contents, unicode) |
1017 | + mock_urlopen.assert_called_once_with('http://example.com/path/') |
1018 | + |
1019 | + def test_no_content_type(self): |
1020 | + # The URL contents are decoded with UTF-8 by default. |
1021 | + patch_urlopen = self.patch_urlopen( |
1022 | + contents=b'URL contents: \xf8', # This is not a UTF-8 byte string. |
1023 | + content_type='text/html') |
1024 | + with patch_urlopen as mock_urlopen: |
1025 | + contents = utils.urlread('http://example.com/path/') |
1026 | + self.assertEqual('URL contents: ', contents) |
1027 | + self.assertIsInstance(contents, unicode) |
1028 | mock_urlopen.assert_called_once_with('http://example.com/path/') |
1029 | |
1030 | def test_errors(self): |
1031 | @@ -582,24 +610,4 @@ |
1032 | with self.assertRaises(IOError) as context_manager: |
1033 | utils.urlread('http://example.com/path/') |
1034 | mock_urlopen.assert_called_once_with('http://example.com/path/') |
1035 | - self.assertEqual(message, str(context_manager.exception)) |
1036 | - |
1037 | - |
1038 | -class TestUtf8(unittest.TestCase): |
1039 | - |
1040 | - def test_unicode(self): |
1041 | - # A unicode value is correctly converted. |
1042 | - value = utils.utf8(u'foo') |
1043 | - self.assertIsInstance(value, str) |
1044 | - self.assertEqual('foo', value) |
1045 | - |
1046 | - def test_bytes(self): |
1047 | - # A bytes value is left untouched. |
1048 | - original = 'foo' |
1049 | - value = utils.utf8(original) |
1050 | - self.assertIsInstance(value, str) |
1051 | - self.assertIs(original, value) |
1052 | - |
1053 | - def test_none(self): |
1054 | - # The None value is returned as is. |
1055 | - self.assertIsNone(utils.utf8(None)) |
1056 | + self.assertEqual(message, bytes(context_manager.exception)) |
1057 | |
1058 | === modified file 'quickstart/utils.py' |
1059 | --- quickstart/utils.py 2013-12-04 18:55:28 +0000 |
1060 | +++ quickstart/utils.py 2013-12-06 18:31:03 +0000 |
1061 | @@ -16,7 +16,11 @@ |
1062 | |
1063 | """Juju Quickstart utility functions and classes.""" |
1064 | |
1065 | -from __future__ import print_function |
1066 | +from __future__ import ( |
1067 | + print_function, |
1068 | + unicode_literals, |
1069 | +) |
1070 | + |
1071 | import collections |
1072 | import httplib |
1073 | import json |
1074 | @@ -26,9 +30,10 @@ |
1075 | import subprocess |
1076 | import urllib2 |
1077 | |
1078 | -import yaml |
1079 | - |
1080 | -from quickstart import settings |
1081 | +from quickstart import ( |
1082 | + serializers, |
1083 | + settings, |
1084 | +) |
1085 | from quickstart.models import charms |
1086 | |
1087 | |
1088 | @@ -55,7 +60,7 @@ |
1089 | for command in commands: |
1090 | retcode, _, error = call('sudo', *command) |
1091 | if retcode: |
1092 | - raise OSError(error) |
1093 | + raise OSError(error.encode('utf-8')) |
1094 | |
1095 | |
1096 | def call(command, *args): |
1097 | @@ -79,7 +84,7 @@ |
1098 | retcode = process.poll() |
1099 | logging.debug('retcode: {} | output: {!r} | error: {!r}'.format( |
1100 | retcode, output, error)) |
1101 | - return retcode, output, error |
1102 | + return retcode, output.decode('utf-8'), error.decode('utf-8') |
1103 | |
1104 | |
1105 | def check_gui_charm_url(charm_url): |
1106 | @@ -103,13 +108,14 @@ |
1107 | |
1108 | |
1109 | def convert_bundle_url(bundle_url): |
1110 | - """Return the equivalent YAML HTTPS location for the fiven bundle URL. |
1111 | + """Return the equivalent YAML HTTPS location for the given bundle URL. |
1112 | |
1113 | Raise a ValueError if the given URL is not a valid bundle URL. |
1114 | """ |
1115 | bundle_id = bundle_url.split(':', 1)[1].rstrip('/') |
1116 | if not bundle_id: |
1117 | - raise ValueError('invalid bundle URL: {!r}'.format(bundle_url)) |
1118 | + msg = 'invalid bundle URL: {}'.format(bundle_url) |
1119 | + raise ValueError(msg.encode('utf-8')) |
1120 | return ('https://manage.jujucharms.com/bundle/{}/json'.format(bundle_id), |
1121 | bundle_id) |
1122 | |
1123 | @@ -123,7 +129,7 @@ |
1124 | charm_info = json.loads(urlread(settings.CHARMWORLD_API)) |
1125 | charm_url = charm_info.get('charm', {}).get('url') |
1126 | if charm_url is None: |
1127 | - raise ValueError('unable to find the charm URL') |
1128 | + raise ValueError(b'unable to find the charm URL') |
1129 | return charm_url |
1130 | |
1131 | |
1132 | @@ -159,7 +165,7 @@ |
1133 | """ |
1134 | retcode, output, error = call('lsb_release', '-cs') |
1135 | if retcode: |
1136 | - raise OSError(error) |
1137 | + raise OSError(error.encode('utf-8')) |
1138 | return output.strip() |
1139 | |
1140 | |
1141 | @@ -182,43 +188,46 @@ |
1142 | """ |
1143 | # Parse the bundle file. |
1144 | try: |
1145 | - bundles = yaml.safe_load(bundle_yaml) |
1146 | + bundles = serializers.yaml_load(bundle_yaml) |
1147 | except Exception as err: |
1148 | - msg = 'unable to parse the bundle: {}'.format(err) |
1149 | + msg = b'unable to parse the bundle: {}'.format(err) |
1150 | raise ValueError(msg) |
1151 | # Ensure the bundle file is well formed and contains at least one bundle. |
1152 | if not isinstance(bundles, collections.Mapping): |
1153 | - raise ValueError('invalid YAML contents: {!r}'.format(bundles)) |
1154 | + msg = 'invalid YAML contents: {}'.format(bundle_yaml) |
1155 | + raise ValueError(msg.encode('utf-8')) |
1156 | try: |
1157 | name_services_map = dict( |
1158 | (key, value['services'].keys()) |
1159 | for key, value in bundles.items() |
1160 | ) |
1161 | except (AttributeError, KeyError, TypeError): |
1162 | - raise ValueError('invalid YAML contents: {!r}'.format(bundles)) |
1163 | + msg = 'invalid YAML contents: {}'.format(bundle_yaml) |
1164 | + raise ValueError(msg.encode('utf-8')) |
1165 | if not name_services_map: |
1166 | - raise ValueError('no bundles found') |
1167 | + raise ValueError(b'no bundles found') |
1168 | # Retrieve the bundle name and services. |
1169 | if bundle_name is None: |
1170 | if len(name_services_map) > 1: |
1171 | msg = 'multiple bundles found ({}) but no bundle name specified' |
1172 | bundle_names = ', '.join(sorted(name_services_map.keys())) |
1173 | - raise ValueError(msg.format(bundle_names)) |
1174 | + raise ValueError(msg.format(bundle_names).encode('utf-8')) |
1175 | bundle_name, bundle_services = name_services_map.items()[0] |
1176 | else: |
1177 | bundle_services = name_services_map.get(bundle_name) |
1178 | if bundle_services is None: |
1179 | msg = 'bundle {} not found in the provided list of bundles ({})' |
1180 | bundle_names = ', '.join(sorted(name_services_map.keys())) |
1181 | - raise ValueError(msg.format(bundle_name, bundle_names)) |
1182 | + raise ValueError( |
1183 | + msg.format(bundle_name, bundle_names).encode('utf-8')) |
1184 | if not bundle_services: |
1185 | msg = 'bundle {} does not include any services'.format(bundle_name) |
1186 | - raise ValueError(msg) |
1187 | + raise ValueError(msg.encode('utf-8')) |
1188 | if settings.JUJU_GUI_SERVICE_NAME in bundle_services: |
1189 | - raise ValueError('bundle {} contains an instance of juju-gui. ' |
1190 | - 'quickstart will install the latest version of the ' |
1191 | - 'Juju GUI automatically, please remove juju-gui from ' |
1192 | - 'the bundle.'.format(bundle_name)) |
1193 | + msg = ('bundle {} contains an instance of juju-gui. quickstart will ' |
1194 | + 'install the latest version of the Juju GUI automatically, ' |
1195 | + 'please remove juju-gui from the bundle.'.format(bundle_name)) |
1196 | + raise ValueError(msg.encode('utf-8')) |
1197 | return bundle_name, bundle_services |
1198 | |
1199 | |
1200 | @@ -231,18 +240,20 @@ |
1201 | if keys is None: |
1202 | keys = ['dummy'] |
1203 | try: |
1204 | - status = yaml.safe_load(output) |
1205 | + status = serializers.yaml_load(output) |
1206 | except Exception as err: |
1207 | - raise ValueError('unable to parse the output: {}'.format(err)) |
1208 | + raise ValueError(b'unable to parse the output: {}'.format(err)) |
1209 | |
1210 | selection = status |
1211 | for key in keys: |
1212 | try: |
1213 | selection = selection.get(key, {}) |
1214 | except AttributeError as err: |
1215 | - raise ValueError('invalid YAML contents: {}'.format(status)) |
1216 | + msg = 'invalid YAML contents: {}'.format(status) |
1217 | + raise ValueError(msg.encode('utf-8')) |
1218 | if selection == {}: |
1219 | - raise ValueError('{} not found in {}'.format(':'.join(keys), status)) |
1220 | + msg = '{} not found in {}'.format(':'.join(keys), status) |
1221 | + raise ValueError(msg.encode('utf-8')) |
1222 | return selection |
1223 | |
1224 | |
1225 | @@ -286,15 +297,12 @@ |
1226 | except urllib2.URLError as err: |
1227 | raise IOError(err.reason) |
1228 | except (httplib.HTTPException, socket.error, urllib2.HTTPError) as err: |
1229 | - raise IOError(str(err)) |
1230 | - return response.read() |
1231 | - |
1232 | - |
1233 | -def utf8(value): |
1234 | - """Return the utf8 encoded version of the given value. |
1235 | - |
1236 | - The given value is returned as is if already encoded or not a string. |
1237 | - """ |
1238 | - if isinstance(value, unicode): |
1239 | - return value.encode('utf-8') |
1240 | - return value |
1241 | + raise IOError(bytes(err)) |
1242 | + contents = response.read() |
1243 | + content_type = response.headers['content-type'] |
1244 | + charset = 'utf-8' |
1245 | + if 'charset=' in content_type: |
1246 | + sent_charset = content_type.split('charset=')[-1].strip() |
1247 | + if sent_charset: |
1248 | + charset = sent_charset |
1249 | + return contents.decode(charset, 'ignore') |
Reviewers: mp+198112_ code.launchpad. net,
Message:
Please take a look.
Description:
Use unicode everywhere in the application.
My apologies for this branch, the diff is long
and the contents themself are boring: I am very
sorry. Anyway, I think this pushes quickstart
in a better place re: unicode/bytes handling.
Everything here is almost mechanical, including
the annoying problem of having to store encoded
strings in the exceptions: please let me know
if you have any suggestions, or if I am missing
something.
The serializers module is new stuff: it allows
for loading YAML strings as unicode and for
dumping dictionaries in the extended format
(used e.g. by juju-core in the envs.yaml file).
To QA this, just use quickstart as usual, deploying
a bundle or spinning up an environment.
Then try to break it passing exotic strings as
options, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url ☢
(FWIW the above character is \xe2\x98\xa2).
This obviously fail, but the error should not
be unicode related.
Thank you and sorry again.
https:/ /code.launchpad .net/~frankban/ juju-quickstart /unicode- all-the- things/ +merge/ 198112
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/38500043/
Affected files (+372, -148 lines): __init_ _.py manage. py models/ charms. py models/ envs.py serializers. py settings. py tests/helpers. py tests/models/ __init_ _.py tests/models/ test_charms. py tests/models/ test_envs. py tests/test_ app.py tests/test_ juju.py tests/test_ manage. py tests/test_ serializers. py tests/test_ utils.py
A [revision details]
M juju-quickstart
M quickstart/
M quickstart/app.py
M quickstart/juju.py
M quickstart/
M quickstart/
M quickstart/
A quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
A quickstart/
M quickstart/
M quickstart/utils.py