Merge lp:~jimbaker/pyjuju/format-2-raw into lp:pyjuju

Proposed by Jim Baker
Status: Merged
Approved by: Kapil Thangavelu
Approved revision: 576
Merged at revision: 576
Proposed branch: lp:~jimbaker/pyjuju/format-2-raw
Merge into: lp:pyjuju
Diff against target: 661 lines (+284/-147)
8 files modified
juju/charm/config.py (+1/-1)
juju/control/tests/test_config_set.py (+1/-22)
juju/hooks/cli.py (+17/-2)
juju/hooks/protocol.py (+0/-1)
juju/hooks/tests/test_cli.py (+7/-24)
juju/hooks/tests/test_invoker.py (+230/-40)
juju/lib/format.py (+17/-24)
juju/lib/tests/test_format.py (+11/-33)
To merge this branch: bzr merge lp:~jimbaker/pyjuju/format-2-raw
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+122380@code.launchpad.net

Description of the change

Modify format 2 so that it supports raw strings

This branch modifies the current format 2 support as follows:

config-get/set:
 - type int: Reads "1"; Writes "1" (no quotes)
 - type float: Reads "1" or "1.0"; Writes "1.0" (no quotes)
 - type boolean: Reads lower(v) in "true" or "false"; Writes "true" or "false"
 - type string: Reads raw data; Writes raw data

relation-get/set
 - String: Reads raw data; Writes raw data

relation-get -
 - YAML output with string keys and raw data values (no nesting!)

 - JSON output with string keys and raw data values (no nesting!) -
   however, high byte strings that are not legal Unicode are base64
   encoded. This should be the same as seen in golang and seems to be
   a fairly common standard

I have tested this with format 1 and format 2 charms interoperating,
as well as various scenarios of raw string input/ouput to ensure that
no extraneous bytes are added, or bytes are lost, and have added
appropriate unit tests. In particular, this branch ensures the support
of null bytes in raw strings. Note that bash can be tricky to use with
null bytes, but this is not a limitation of Juju itself.

https://codereview.appspot.com/6490069/

To post a comment you must log in.
Revision history for this message
Jim Baker (jimbaker) wrote :

Reviewers: mp+122380_code.launchpad.net,

Message:
Please take a look.

Description:
Modify format 2 so that it supports raw strings

This branch modifies the current format 2 support as follows:

config-get/set:
  - type int: Reads "1"; Writes "1" (no quotes)
  - type float: Reads "1" or "1.0"; Writes "1.0" (no quotes)
  - type boolean: Reads lower(v) in "true" or "false"; Writes "true" or
"false"
  - type string: Reads raw data; Writes raw data

relation-get/set
  - String: Reads raw data; Writes raw data

relation-get -
  - YAML output with string keys and raw data values (no nesting!)

  - JSON output with string keys and raw data values (no nesting!) -
    however, high byte strings that are not legal Unicode are base64
    encoded. This should be the same as seen in golang and seems to be
    a fairly common standard

I have tested this with format 1 and format 2 charms interoperating,
as well as various scenarios of raw string input/ouput to ensure that
no extraneous bytes are added, or bytes are lost, and have added
appropriate unit tests. In particular, this branch ensures the support
of null bytes in raw strings. Note that bash can be tricky to use with
null bytes, but this is not a limitation of Juju itself.

https://code.launchpad.net/~jimbaker/juju/format-2-raw/+merge/122380

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/charm/config.py
   M juju/control/tests/test_config_set.py
   M juju/hooks/cli.py
   M juju/hooks/protocol.py
   M juju/hooks/tests/test_cli.py
   M juju/hooks/tests/test_invoker.py
   M juju/lib/format.py
   M juju/lib/tests/test_format.py

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

looks great, +1

https://codereview.appspot.com/6490069/diff/1/juju/hooks/cli.py
File juju/hooks/cli.py (right):

https://codereview.appspot.com/6490069/diff/1/juju/hooks/cli.py#newcode233
juju/hooks/cli.py:233: # encoded; workaround by firt testing whether it
can be
s/firt/first

https://codereview.appspot.com/6490069/

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Can we please document this in lp:juju/docs before merging it?

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

Thats an independent task and branch that should follow. At the moment no
one is using format-2, and we should have the right implementation in place
b4 documenting usage.

On Tue, Sep 4, 2012 at 7:02 PM, Clint Byrum <email address hidden> wrote:

> Can we please document this in lp:juju/docs before merging it?
> --
> https://code.launchpad.net/~jimbaker/juju/format-2-raw/+merge/122380
> You are subscribed to branch lp:juju.
>

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Excerpts from Kapil Thangavelu's message of 2012-09-06 18:01:26 UTC:
> Thats an independent task and branch that should follow. At the moment no
> one is using format-2, and we should have the right implementation in place
> b4 documenting usage.

I see it a bit different, in that I'd like to see a specification
somewhere before the implementation is merged (not worked on.. just
before it is merged).

Either way, documentation of the format needs to be done before we cut
a 0.6 release.

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

On Thu, Sep 6, 2012 at 5:31 PM, Clint Byrum <email address hidden> wrote:

> Excerpts from Kapil Thangavelu's message of 2012-09-06 18:01:26 UTC:
> > Thats an independent task and branch that should follow. At the moment no
> > one is using format-2, and we should have the right implementation in
> place
> > b4 documenting usage.
>
> I see it a bit different, in that I'd like to see a specification
> somewhere before the implementation is merged (not worked on.. just
> before it is merged).
>
>
This has been the subject of at least three threads on list, with the most
recent providing an exact specification of the delta.

> Either way, documentation of the format needs to be done before we cut
> a 0.6 release.
>

Agreed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/charm/config.py'
2--- juju/charm/config.py 2012-07-02 03:14:51 +0000
3+++ juju/charm/config.py 2012-09-01 05:47:20 +0000
4@@ -144,7 +144,7 @@
5 # output format, False
6 raise ServiceConfigValueError(
7 "Invalid value for %s: %s" % (
8- name, YAMLFormat().format(value)))
9+ name, YAMLFormat().format_raw(value)))
10 return value
11
12 def get_defaults(self):
13
14=== modified file 'juju/control/tests/test_config_set.py'
15--- juju/control/tests/test_config_set.py 2012-07-02 23:55:27 +0000
16+++ juju/control/tests/test_config_set.py 2012-09-01 05:47:20 +0000
17@@ -261,7 +261,7 @@
18 self.setup_exit(0)
19 self.mocker.replay()
20 main(["set", "mysql-format-v2",
21- "monkey-madness='barrels of monkeys'"])
22+ "monkey-madness=barrels of monkeys"])
23 yield finished
24 self.assertEqual(
25 self.output.getvalue(),
26@@ -290,24 +290,3 @@
27 state,
28 {"awesome": False, "monkey-madness": 0.5,
29 "query-cache-size": -1, "tuning-level": "safest"})
30-
31- @inlineCallbacks
32- def test_invalid_string_option_format_v2(self):
33- """Verify that config settings reject invalid string"""
34- self.service_state = yield self.add_service_from_charm(
35- "mysql-format-v2")
36- finished = self.setup_cli_reactor()
37- self.setup_exit(0)
38- self.mocker.replay()
39- # YAML does a fair amount of coercion... note that it needs to
40- # be quoted if it looks like a boolean/int/string
41- main(["set", "mysql-format-v2", "tuning-level=FALSE"])
42- yield finished
43- self.assertEqual(
44- self.output.getvalue(),
45- "Invalid value for tuning-level: false\n")
46- state = yield self.service_state.get_config()
47- self.assertEqual(
48- state,
49- {"awesome": False, "monkey-madness": 0.5,
50- "query-cache-size": -1, "tuning-level": "safest"})
51
52=== modified file 'juju/hooks/cli.py'
53--- juju/hooks/cli.py 2012-06-14 14:33:22 +0000
54+++ juju/hooks/cli.py 2012-09-01 05:47:20 +0000
55@@ -1,4 +1,6 @@
56 import argparse
57+import base64
58+import copy
59 import json
60 import logging
61 import os
62@@ -223,12 +225,25 @@
63 stream.flush()
64
65 def format_json(self, result, stream):
66- print >>stream, json.dumps(result)
67+ encoded = copy.copy(result)
68+ if isinstance(result, dict):
69+ for k, v in result.iteritems():
70+ # Workaround the fact that JSON does not work with str
71+ # values that have high bytes and are not actually UTF-8
72+ # encoded; workaround by firt testing whether it can be
73+ # decoded as UTF-8, and if not, wrapping as Base64
74+ # encoded.
75+ if isinstance(v, str):
76+ try:
77+ v.decode("utf8")
78+ except UnicodeDecodeError:
79+ encoded[k] = base64.b64encode(v)
80+ json.dump(encoded, stream)
81
82 def format_smart(self, result, stream):
83 if result is not None:
84 charm_formatter = get_charm_formatter_from_env()
85- print >>stream, charm_formatter.format(result)
86+ stream.write(charm_formatter.format_raw(result))
87
88
89 def parse_log_level(level):
90
91=== modified file 'juju/hooks/protocol.py'
92--- juju/hooks/protocol.py 2012-07-03 00:39:39 +0000
93+++ juju/hooks/protocol.py 2012-09-01 05:47:20 +0000
94@@ -335,7 +335,6 @@
95 # status reporting
96 yield container.open_port(port, proto)
97
98-
99 yield service_unit_state.open_port(port, proto)
100 yield self.factory.log(logging.DEBUG, "opened %s/%s" % (port, proto))
101 defer.returnValue({})
102
103=== modified file 'juju/hooks/tests/test_cli.py'
104--- juju/hooks/tests/test_cli.py 2012-06-15 01:16:13 +0000
105+++ juju/hooks/tests/test_cli.py 2012-09-01 05:47:20 +0000
106@@ -8,7 +8,6 @@
107 from contextlib import closing
108
109 from twisted.internet.defer import inlineCallbacks, returnValue
110-import yaml
111
112 from juju.hooks.cli import (
113 CommandLineClient, parse_log_level, parse_port_protocol)
114@@ -423,39 +422,23 @@
115 with closing(StringIO.StringIO()) as output:
116 cli.format_smart(sample, output)
117 self.assertEqual(output.getvalue(), formatted)
118- self.assertEqual(sample, yaml.safe_load(output.getvalue()))
119
120 def test_format_smart_v2(self):
121- """Verifies smart format v2 writes correct YAML"""
122+ """Verifies smart format v2 writes raw strings properly"""
123 self.change_environment(_JUJU_CHARM_FORMAT="2")
124
125 # For each case, verify actual output serialization along with
126 # roundtripping through YAML
127 self.assert_smart_output(None, "") # No newline in output for None
128- self.assert_smart_output("", "''\n")
129- self.assert_smart_output("A string", "A string\n")
130- # Note: YAML uses b64 encoding for byte strings tagged by !!binary
131- self.assert_smart_output(
132- "High bytes: \xCA\xFE",
133- "!!binary |\n SGlnaCBieXRlczogyv4=\n")
134- self.assert_smart_output(u"", "''\n")
135- self.assert_smart_output(
136- u"A unicode string (but really ascii)",
137- "A unicode string (but really ascii)\n")
138- # Any non-ascii Unicode will use UTF-8 encoding
139- self.assert_smart_output(u"中文", "\xe4\xb8\xad\xe6\x96\x87\n")
140- self.assert_smart_output({}, "{}\n")
141+ self.assert_smart_output("", "")
142+ self.assert_smart_output("A string", "A string")
143+ self.assert_smart_output(
144+ "High bytes: \xCA\xFE", "High bytes: \xca\xfe")
145+ self.assert_smart_output("中文", "\xe4\xb8\xad\xe6\x96\x87")
146 self.assert_smart_output(
147 {u"public-address": u"ec2-1-2-3-4.compute-1.amazonaws.com",
148 u"foo": u"bar",
149 u"configured": True},
150 ("configured: true\n"
151 "foo: bar\n"
152- "public-address: ec2-1-2-3-4.compute-1.amazonaws.com\n"))
153- self.assert_smart_output(False, "false\n")
154- self.assert_smart_output(True, "true\n")
155- self.assert_smart_output(0.0, "0.0\n")
156- self.assert_smart_output(3.14159, "3.14159\n")
157- self.assert_smart_output(6.02214178e23, "6.02214178e+23\n")
158- self.assert_smart_output(0, "0\n")
159- self.assert_smart_output(42, "42\n")
160+ "public-address: ec2-1-2-3-4.compute-1.amazonaws.com"))
161
162=== modified file 'juju/hooks/tests/test_invoker.py'
163--- juju/hooks/tests/test_invoker.py 2012-07-03 07:30:53 +0000
164+++ juju/hooks/tests/test_invoker.py 2012-09-01 05:47:20 +0000
165@@ -1,6 +1,7 @@
166 # -*- encoding: utf-8 -*-
167
168 from StringIO import StringIO
169+import base64
170 import json
171 import logging
172 import os
173@@ -192,7 +193,7 @@
174
175 def create_hook(self, hook, arguments):
176 bin_path = self.get_cli_hook(hook)
177- fn = self.makeFile("#!/bin/sh\n'%s' %s" % (bin_path, arguments))
178+ fn = self.makeFile("#!/bin/bash\n'%s' %s" % (bin_path, arguments))
179 # make the hook executable
180 os.chmod(fn, stat.S_IEXEC | stat.S_IREAD)
181 return fn
182@@ -1516,7 +1517,6 @@
183 # we don't see units in the other container
184 self.assertNotIn("mysql/0", self.log.getvalue())
185
186-
187 @defer.inlineCallbacks
188 def test_open_and_close_ports(self):
189 """Verify that port hook commands run and changes are immediate."""
190@@ -1547,15 +1547,14 @@
191 [{"port": 80, "proto": "tcp"},
192 {"port": 53, "proto": "udp"}])
193
194-
195 result = yield exe(self.create_hook("close-port", "80/tcp"))
196 self.assertEqual(result, 0)
197 self.assertEqual(
198 (yield unit_state.get_open_ports()),
199- [{"port": 53, "proto": "udp"} ,])
200+ [{"port": 53, "proto": "udp"}])
201 self.assertEqual(
202 (yield container_state.get_open_ports()),
203- [{"port": 53, "proto": "udp"},])
204+ [{"port": 53, "proto": "udp"}])
205
206 yield exe.ended
207 self.assertLogLines(
208@@ -1658,6 +1657,16 @@
209 "mysql", charm_name="mysql-format-v2")
210 yield super(TestCharmFormatV2, self)._default_relations()
211
212+ def make_zipped_file(self):
213+ data_file = self.makeFile()
214+ with open(data_file, "wb") as f:
215+ # gzipped of 'abc' - however gzip will also includes the
216+ # source file name, so easiest to keep it stable here as
217+ # standard data
218+ f.write("\x1f\x8b\x08\x08\xbb\x8bAP\x02\xfftmpr"
219+ "fyP0e\x00KLJ\x06\x00\xc2A$5\x03\x00\x00\x00")
220+ return data_file
221+
222 @defer.inlineCallbacks
223 def test_environment(self):
224 """Ensure that an explicit setting of format: 2 works properly"""
225@@ -1667,7 +1676,7 @@
226 self.assertEqual(env["_JUJU_CHARM_FORMAT"], "2")
227
228 @defer.inlineCallbacks
229- def test_output(self):
230+ def test_smart_output(self):
231 """Verify roundtripping"""
232 hook_debug_log = capture_separate_log("hook", level=logging.DEBUG)
233 hook_log = capture_separate_log("hook", level=logging.INFO)
234@@ -1675,54 +1684,235 @@
235 "database:42", "add", "mysql/0", self.relation,
236 client_id="client_id")
237
238- # Byte strings are also supported by staying completely in
239- # YAML, so test that corner case. This also means users need
240- # to present valid YAML for any input:
241- data_file = self.makeFile(
242- yaml.safe_dump("But when I do drink, I prefer \xCA\xFE"))
243+ # Test the support of raw strings, both from a file and from
244+ # command line. Unicode can also be used - this is just
245+ # rendered as UTF-8 in the shell; the source here is also
246+ # UTF-8 - note it is not a Unicode string, it's a bytestring.
247+ data_file = self.make_zipped_file()
248 set_hook = self.create_hook(
249 "relation-set",
250- "b=true i=42 f=1.23 s=ascii u=中文 d=@%s" % data_file)
251+ "b=true f=1.23 i=42 s=ascii u=中文 d=@%s "
252+ "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
253+ data_file))
254 yield exe(set_hook)
255+
256 result = yield exe(self.create_hook("relation-get", "- mysql/0"))
257 self.assertEqual(result, 0)
258
259- # YAML guarantees that the keys will be sorted
260- # lexicographically; note that we output UTF-8 for Unicode
261- # when dumping YAML, so our source text (with this test file
262- # in UTF-8 itself) matches the output text, as seen in the
263- # characters for "zhongwen" (Chinese language).
264+ # relation-get - uses YAML to dump keys. YAML guarantees that
265+ # the keys will be sorted lexicographically; note that we
266+ # output UTF-8 for Unicode when dumping YAML, so our source
267+ # text (with this test file in UTF-8 itself) matches the
268+ # output text, as seen in the characters for "zhongwen"
269+ # (Chinese language).
270 self.assertEqual(
271 hook_log.getvalue(),
272- "b: true\n"
273- "d: !!binary |\n QnV0IHdoZW4gSSBkbyBkcmluaywgSSBwcmVmZXIgyv4=\n"
274- "f: 1.23\n"
275- "i: 42\n"
276+ "b: 'true'\n"
277+ "d: !!binary |\n H4sICLuLQVAC/3RtcHJmeVAwZQBLTEoGAMJBJDUDAAAA\n"
278+ "f: '1.23'\n"
279+ "i: '42'\n"
280 "private-address: mysql-0.example.com\n"
281+ "r: !!binary |\n QnV0IHdoZW4gSSBkbyBkcmluaywgSSBwcmVmZXIgyv4=\n"
282 "s: ascii\n"
283- "u: 中文\n\n")
284+ "u: 中文\n")
285
286- # Log lines are not simply converted into Unicode, as in v1 format
287+ # Note: backslashes are necessarily doubled here; r"XYZ"
288+ # strings don't help with hexescapes
289 self.assertLogLines(
290 hook_debug_log.getvalue(),
291 ["Flushed values for hook %r on 'database:42'" % (
292 os.path.basename(set_hook),),
293- " Setting changed: 'b'=True (was unset)",
294- " Setting changed: 'd'='But when I do drink, "
295+ " Setting changed: 'b'='true' (was unset)",
296+ " Setting changed: 'd'='\\x1f\\x8b\\x08\\x08\\xbb\\x8bAP\\x02"
297+ "\\xfftmprfyP0e\\x00KLJ\\x06\\x00\\xc2A$5\\x03"
298+ "\\x00\\x00\\x00' (was unset)",
299+ " Setting changed: 'f'='1.23' (was unset)",
300+ " Setting changed: 'i'='42' (was unset)",
301+ " Setting changed: 'r'='But when I do drink, "
302 "I prefer \\xca\\xfe' (was unset)",
303- " Setting changed: 'f'=1.23 (was unset)",
304- " Setting changed: 'i'=42 (was unset)",
305 " Setting changed: 's'='ascii' (was unset)",
306- " Setting changed: 'u'=u'\\u4e2d\\u6587' (was unset)"])
307-
308- # Also ensure that invalid YAML is rejected; unlike earlier,
309- # this was not encoded with yaml.safe_dump
310- data_file = self.makeFile(
311- "But when I do drink, I prefer \xCA\xFE")
312- hook = self.create_hook("relation-set", "d=@%s" % data_file)
313- e = yield self.assertFailure(exe(hook), errors.CharmInvocationError)
314- self.assertEqual(str(e), "Error processing %r: exit code 1." % hook)
315- self.assertIn(
316- "yaml.reader.ReaderError: \'utf8\' codec can\'t decode byte #xca: "
317- "invalid continuation byte\n in \"<string>\", position 30",
318- hook_log.getvalue())
319+ " Setting changed: 'u'=u'\\u4e2d\\u6587' (was unset)"
320+ ])
321+
322+ @defer.inlineCallbacks
323+ def test_exact_roundtrip_binary_data(self):
324+ """Verify that binary data, including \x00, is roundtripped exactly"""
325+ hook_log = capture_separate_log("hook", level=logging.INFO)
326+ exe = yield self.ua.get_invoker(
327+ "database:42", "add", "mysql/0", self.relation,
328+ client_id="client_id")
329+ data_file = self.make_zipped_file()
330+
331+ # relation-set can only read null bytes from a file; bash
332+ # would otherwise silently drop
333+ set_hook = self.create_hook("relation-set", "zipped=@%s" % (
334+ data_file))
335+ result = yield exe(set_hook)
336+ self.assertEqual(result, 0)
337+
338+ # Abuse the create_hook method a little bit by adding a pipe
339+ get_hook = self.create_hook("relation-get", "zipped mysql/0 | zcat")
340+ result = yield exe(get_hook)
341+ self.assertEqual(result, 0)
342+
343+ # Using the hook log for this verification does generate one
344+ # extra \n (seen elsewhere in our tests), but this is just
345+ # test noise: we are guaranteed roundtrip fidelity by using
346+ # the picky tool that is zcat - no extraneous data accepted.
347+ self.assertEqual(hook_log.getvalue(), "abc\n")
348+
349+ @defer.inlineCallbacks
350+ def test_json_output(self):
351+ """Verify roundtripping"""
352+ hook_log = capture_separate_log("hook", level=logging.INFO)
353+ exe = yield self.ua.get_invoker(
354+ "database:42", "add", "mysql/0", self.relation,
355+ client_id="client_id")
356+
357+ # Test the support of raw strings, both from a file and from
358+ # command line. In addition, test Unicode indirectly by using
359+ # UTF-8. Because the source of this file is marked as UTF-8,
360+ # we can embed such characters directly in bytestrings, not
361+ # just Unicode strings. This also works within the context of
362+ # the shell.
363+ raw = "But when I do drink, I prefer \xca\xfe"
364+ data_file = self.makeFile(raw)
365+ set_hook = self.create_hook(
366+ "relation-set",
367+ "b=true f=1.23 i=42 s=ascii u=中文 d=@%s "
368+ "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
369+ data_file,))
370+ yield exe(set_hook)
371+
372+ result = yield exe(self.create_hook(
373+ "relation-get", "--format=json - mysql/0"))
374+ self.assertEqual(result, 0)
375+
376+ # YAML serialization internally has converted (transparently)
377+ # UTF-8 to Unicode, which can be rendered by JSON. However the
378+ # "cafe" bytestring is invalid JSON, so verify that it's been
379+ # Base64 encoded.
380+ encoded = base64.b64encode(raw)
381+ self.assertEqual(
382+ hook_log.getvalue(),
383+ '{"b": "true", '
384+ '"d": "%s", '
385+ '"f": "1.23", '
386+ '"i": "42", '
387+ '"private-address": "mysql-0.example.com", '
388+ '"s": "ascii", '
389+ '"r": "%s", '
390+ '"u": "\\u4e2d\\u6587"}\n' % (encoded, encoded))
391+
392+ @defer.inlineCallbacks
393+ def common_relation_set(self):
394+ hook_log = capture_separate_log("hook", level=logging.INFO)
395+ exe = yield self.ua.get_invoker(
396+ "database:42", "add", "mysql/0",
397+ self.relation, client_id="client_id")
398+ raw = "But when I do drink, I prefer \xCA\xFE"
399+ data_file = self.makeFile(raw)
400+ set_hook = self.create_hook(
401+ "relation-set",
402+ "s='some text' u=中文 d=@%s "
403+ "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
404+ data_file))
405+ result = yield exe(set_hook)
406+ self.assertEqual(result, 0)
407+ defer.returnValue((exe, hook_log))
408+
409+ @defer.inlineCallbacks
410+ def test_relation_get_ascii(self):
411+ """Verify that ascii data is roundtripped"""
412+ exe, hook_log = yield self.common_relation_set()
413+ result = yield exe(self.create_hook("relation-get", "s mysql/0"))
414+ self.assertEqual(result, 0)
415+ self.assertEqual(hook_log.getvalue(), "some text\n")
416+
417+ @defer.inlineCallbacks
418+ def test_relation_get_raw(self):
419+ """Verify that raw data is roundtripped"""
420+ exe, hook_log = yield self.common_relation_set()
421+ result = yield exe(self.create_hook("relation-get", "r mysql/0"))
422+ self.assertEqual(result, 0)
423+ self.assertEqual(
424+ hook_log.getvalue(), "But when I do drink, I prefer \xca\xfe\n")
425+
426+ @defer.inlineCallbacks
427+ def test_relation_get_unicode(self):
428+ """Verify Unicode is roundtripped (via UTF-8) through the shell"""
429+ exe, hook_log = yield self.common_relation_set()
430+
431+ result = yield exe(self.create_hook("relation-get", "u mysql/0"))
432+ self.assertEqual(result, 0)
433+ self.assertEqual(hook_log.getvalue(), "中文\n")
434+
435+ @defer.inlineCallbacks
436+ def setup_config(self):
437+ hook_log = self.capture_logging("hook")
438+ exe = yield self.ua.get_invoker(
439+ "db:42", "add", "mysql/0", self.relation, client_id="client_id")
440+ context = yield self.ua.get_context("client_id")
441+ config = yield context.get_config()
442+ with open(self.make_zipped_file(), "rb") as f:
443+ data = f.read()
444+ config.update({
445+ "b": True,
446+ "f": 1.23,
447+ "i": 42,
448+ "s": "some text",
449+ # uses UTF-8 encoding in this test script
450+ "u": "中文",
451+ # use high byte and null byte characters
452+ "r": data
453+ })
454+ yield config.write()
455+ defer.returnValue((exe, hook_log))
456+
457+ @defer.inlineCallbacks
458+ def test_config_get_boolean(self):
459+ """Validate that config-get returns lowercase names of booleans."""
460+ exe, hook_log = yield self.setup_config()
461+ result = yield exe(self.create_hook("config-get", "b"))
462+ self.assertEqual(result, 0)
463+ self.assertEqual(hook_log.getvalue(), "true\n")
464+
465+ @defer.inlineCallbacks
466+ def test_config_get_float(self):
467+ """Validate that config-get returns floats without quotes."""
468+ exe, hook_log = yield self.setup_config()
469+ result = yield exe(self.create_hook("config-get", "f"))
470+ self.assertEqual(result, 0)
471+ self.assertEqual(hook_log.getvalue(), "1.23\n")
472+
473+ @defer.inlineCallbacks
474+ def test_config_get_int(self):
475+ """Validate that config-get returns ints without quotes."""
476+ exe, hook_log = yield self.setup_config()
477+ result = yield exe(self.create_hook("config-get", "i"))
478+ self.assertEqual(result, 0)
479+ self.assertEqual(hook_log.getvalue(), "42\n")
480+
481+ @defer.inlineCallbacks
482+ def test_config_get_ascii(self):
483+ """Validate that config-get returns ascii strings."""
484+ exe, hook_log = yield self.setup_config()
485+ result = yield exe(self.create_hook("config-get", "s"))
486+ self.assertEqual(result, 0)
487+ self.assertEqual(hook_log.getvalue(), "some text\n")
488+
489+ @defer.inlineCallbacks
490+ def test_config_get_raw(self):
491+ """Validate config-get can work with high and null bytes."""
492+ exe, hook_log = yield self.setup_config()
493+ result = yield exe(self.create_hook("config-get", "r | zcat"))
494+ self.assertEqual(result, 0)
495+ self.assertEqual(hook_log.getvalue(), "abc\n")
496+
497+ @defer.inlineCallbacks
498+ def test_config_get_unicode(self):
499+ """Validate that config-get returns raw strings containing UTF-8."""
500+ exe, hook_log = yield self.setup_config()
501+ result = yield exe(self.create_hook("config-get", "u"))
502+ self.assertEqual(result, 0)
503+ self.assertEqual(hook_log.getvalue(), "中文\n")
504
505=== modified file 'juju/lib/format.py'
506--- juju/lib/format.py 2012-06-22 19:08:23 +0000
507+++ juju/lib/format.py 2012-09-01 05:47:20 +0000
508@@ -42,6 +42,14 @@
509
510 return data
511
512+ def _parse_value(self, key, value):
513+ """Interprets value as a str"""
514+ return value
515+
516+ def should_delete(self, value):
517+ """Whether `value` implies corresponding key should be deleted"""
518+ return not value.strip()
519+
520
521 class PythonFormat(BaseFormat):
522 """Supports backwards compatibility through str and JSON encoding."""
523@@ -52,9 +60,9 @@
524 """Formats `data` using Python str encoding"""
525 return str(data)
526
527- def _parse_value(self, key, value):
528- """Interprets value as a str"""
529- return value
530+ def format_raw(self, data):
531+ """Add extra \n seen in Python format, so not truly raw"""
532+ return self.format(data) + "\n"
533
534 # For the old format: 1, using JSON serialization introduces some
535 # subtle issues around Unicode conversion that then later results
536@@ -69,13 +77,6 @@
537 """Loads data, but also converts str to Unicode"""
538 return json.loads(data)
539
540- def should_delete(self, value):
541- """Whether `value` implies corresponding key should be deleted"""
542- # In format: 1, all values are strings, but possibly with
543- # spaces. The strip reduces strings consisting only of spaces,
544- # or otherwise empty, to an empty string.
545- return not value.strip()
546-
547
548 class YAMLFormat(BaseFormat):
549 """New format that uses YAML, so no unexpected encoding issues"""
550@@ -98,13 +99,12 @@
551 # Also remove any extra \n, will still be valid yaml
552 return serialized.rstrip("\n")
553
554- def _parse_value(self, key, value):
555- # Ensure roundtripping to/from yaml if format: 2; in
556- # particular this ensures that true/false work as desired
557- try:
558- return yaml.safe_load(value)
559- except Exception:
560- raise JujuError("Invalid YAML value (argument:%s)" % key)
561+ def format_raw(self, data):
562+ """Formats `data` as a raw string if str, otherwise as YAML"""
563+ if isinstance(data, str):
564+ return data
565+ else:
566+ return self.format(data)
567
568 # Use the same format for dump
569 dump = format
570@@ -113,13 +113,6 @@
571 """Loads data safely, ensuring no Python specific type info leaks"""
572 return yaml.safe_load(data)
573
574- def should_delete(self, value):
575- """Whether `value` implies corresponding key should be deleted"""
576- # In format: 2, values were already parsed by yaml.safe_load;
577- # in particular, a string consisting only of whitespace is
578- # parsed as None.
579- return value is None
580-
581
582 def is_valid_charm_format(charm_format):
583 """True if `charm_format` is a valid format"""
584
585=== modified file 'juju/lib/tests/test_format.py'
586--- juju/lib/tests/test_format.py 2012-06-22 19:08:23 +0000
587+++ juju/lib/tests/test_format.py 2012-09-01 05:47:20 +0000
588@@ -223,7 +223,7 @@
589 def assert_parse(self, data):
590 """Verify input parses as expected, including from a data file"""
591 formatter = YAMLFormat()
592- formatted = formatter.format(data)
593+ formatted = formatter.format_raw(data)
594 data_file = self.makeFile(formatted)
595 kvs = ["formatted=%s" % formatted,
596 "file=@%s" % data_file]
597@@ -234,27 +234,9 @@
598 def test_parse_keyvalue_pairs(self):
599 """Verify key value pairs parse for a wide range of YAML inputs."""
600 formatter = YAMLFormat()
601- self.assert_parse(None)
602 self.assert_parse("")
603 self.assert_parse("A string")
604 self.assert_parse("High bytes: \xCA\xFE")
605- self.assert_parse(u"")
606- self.assert_parse(u"A unicode string (but really ascii)")
607- self.assert_parse(u"中文")
608- self.assert_parse({})
609- self.assert_parse(
610- {u"public-address": u"ec2-1-2-3-4.compute-1.amazonaws.com",
611- u"foo": u"bar",
612- u"configured": True})
613- self.assert_parse([])
614- self.assert_parse(["abc", "xyz", 42, True])
615- self.assert_parse(False)
616- self.assert_parse(True)
617- self.assert_parse(0.0)
618- self.assert_parse(3.14159)
619- self.assert_parse(6.02214178e23)
620- self.assert_parse(0)
621- self.assert_parse(42)
622
623 # Raises an error if no such file
624 e = self.assertRaises(
625@@ -271,14 +253,6 @@
626 self.assertEquals(
627 str(e), "Expected `option=value`. Found `foobar`")
628
629- # Raises an error if the value is invalid YAML
630- e = self.assertRaises(
631- JujuError,
632- formatter.parse_keyvalue_pairs, ["content=\xCA\FE"])
633- self.assertEquals(
634- str(e),
635- "Invalid YAML value (argument:content)")
636-
637 def assert_dump_load(self, data, expected):
638 """Asserts expected formatting and roundtrip through dump/load"""
639 formatter = YAMLFormat()
640@@ -320,11 +294,15 @@
641 self.assert_dump_load(42, "data: 42")
642
643 def test_should_delete(self):
644- """Verify only `None` values (as YAML loaded) indicate deletion"""
645- formatter = YAMLFormat()
646+ """Verify empty or whitespace only strings indicate deletion"""
647+ formatter = PythonFormat()
648 self.assertFalse(formatter.should_delete("0"))
649 self.assertFalse(formatter.should_delete("something"))
650- self.assertFalse(formatter.should_delete(""))
651- self.assertFalse(formatter.should_delete(" "))
652- self.assertFalse(formatter.should_delete(0))
653- self.assertTrue(formatter.should_delete(None))
654+ self.assertTrue(formatter.should_delete(""))
655+ self.assertTrue(formatter.should_delete(" "))
656+
657+ # Verify that format: 1 can only work with str values
658+ e = self.assertRaises(AttributeError, formatter.should_delete, 42)
659+ self.assertEqual(str(e), "'int' object has no attribute 'strip'")
660+ e = self.assertRaises(AttributeError, formatter.should_delete, None)
661+ self.assertEqual(str(e), "'NoneType' object has no attribute 'strip'")

Subscribers

People subscribed via source and target branches

to status/vote changes: