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

[Jim Baker]

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/

[Jim Baker (jimbaker)]

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

[Kapil Thangavelu (hazmat)]

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/

[Clint Byrum (clint-fewbar)]

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

[Kapil Thangavelu (hazmat)]

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.
>

[Clint Byrum (clint-fewbar)]

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.

[Kapil Thangavelu (hazmat)]

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.

=== modified file 'juju/charm/config.py'
--- juju/charm/config.py	2012-07-02 03:14:51 +0000
+++ juju/charm/config.py	2012-09-01 05:47:20 +0000
@@ -144,7 +144,7 @@
             # output format, False
             raise ServiceConfigValueError(
                 "Invalid value for %s: %s" % (
-                    name, YAMLFormat().format(value)))
+                    name, YAMLFormat().format_raw(value)))
         return value
 
     def get_defaults(self):
 
=== modified file 'juju/control/tests/test_config_set.py'
--- juju/control/tests/test_config_set.py	2012-07-02 23:55:27 +0000
+++ juju/control/tests/test_config_set.py	2012-09-01 05:47:20 +0000
@@ -261,7 +261,7 @@
         self.setup_exit(0)
         self.mocker.replay()
         main(["set", "mysql-format-v2",
-              "monkey-madness='barrels of monkeys'"])
+              "monkey-madness=barrels of monkeys"])
         yield finished
         self.assertEqual(
             self.output.getvalue(),
@@ -290,24 +290,3 @@
             state,
             {"awesome": False, "monkey-madness": 0.5,
              "query-cache-size": -1, "tuning-level": "safest"})
-
-    @inlineCallbacks
-    def test_invalid_string_option_format_v2(self):
-        """Verify that config settings reject invalid string"""
-        self.service_state = yield self.add_service_from_charm(
-            "mysql-format-v2")
-        finished = self.setup_cli_reactor()
-        self.setup_exit(0)
-        self.mocker.replay()
-        # YAML does a fair amount of coercion... note that it needs to
-        # be quoted if it looks like a boolean/int/string
-        main(["set", "mysql-format-v2", "tuning-level=FALSE"])
-        yield finished
-        self.assertEqual(
-            self.output.getvalue(),
-            "Invalid value for tuning-level: false\n")
-        state = yield self.service_state.get_config()
-        self.assertEqual(
-            state,
-            {"awesome": False, "monkey-madness": 0.5,
-             "query-cache-size": -1, "tuning-level": "safest"})
 
=== modified file 'juju/hooks/cli.py'
--- juju/hooks/cli.py	2012-06-14 14:33:22 +0000
+++ juju/hooks/cli.py	2012-09-01 05:47:20 +0000
@@ -1,4 +1,6 @@
 import argparse
+import base64
+import copy
 import json
 import logging
 import os
@@ -223,12 +225,25 @@
         stream.flush()
 
     def format_json(self, result, stream):
-        print >>stream, json.dumps(result)
+        encoded = copy.copy(result)
+        if isinstance(result, dict):
+            for k, v in result.iteritems():
+                # Workaround the fact that JSON does not work with str
+                # values that have high bytes and are not actually UTF-8
+                # encoded; workaround by firt testing whether it can be
+                # decoded as UTF-8, and if not, wrapping as Base64
+                # encoded.
+                if isinstance(v, str):
+                    try:
+                        v.decode("utf8")
+                    except UnicodeDecodeError:
+                        encoded[k] = base64.b64encode(v)
+        json.dump(encoded, stream)
 
     def format_smart(self, result, stream):
         if result is not None:
             charm_formatter = get_charm_formatter_from_env()
-            print >>stream, charm_formatter.format(result)
+            stream.write(charm_formatter.format_raw(result))
 
 
 def parse_log_level(level):
 
=== modified file 'juju/hooks/protocol.py'
--- juju/hooks/protocol.py	2012-07-03 00:39:39 +0000
+++ juju/hooks/protocol.py	2012-09-01 05:47:20 +0000
@@ -335,7 +335,6 @@
             # status reporting
             yield container.open_port(port, proto)
 
-
         yield service_unit_state.open_port(port, proto)
         yield self.factory.log(logging.DEBUG, "opened %s/%s" % (port, proto))
         defer.returnValue({})
 
=== modified file 'juju/hooks/tests/test_cli.py'
--- juju/hooks/tests/test_cli.py	2012-06-15 01:16:13 +0000
+++ juju/hooks/tests/test_cli.py	2012-09-01 05:47:20 +0000
@@ -8,7 +8,6 @@
 from contextlib import closing
 
 from twisted.internet.defer import inlineCallbacks, returnValue
-import yaml
 
 from juju.hooks.cli import (
     CommandLineClient, parse_log_level, parse_port_protocol)
@@ -423,39 +422,23 @@
         with closing(StringIO.StringIO()) as output:
             cli.format_smart(sample, output)
             self.assertEqual(output.getvalue(), formatted)
-            self.assertEqual(sample, yaml.safe_load(output.getvalue()))
 
     def test_format_smart_v2(self):
-        """Verifies smart format v2 writes correct YAML"""
+        """Verifies smart format v2 writes raw strings properly"""
         self.change_environment(_JUJU_CHARM_FORMAT="2")
 
         # For each case, verify actual output serialization along with
         # roundtripping through YAML
         self.assert_smart_output(None, "")  # No newline in output for None
-        self.assert_smart_output("", "''\n")
-        self.assert_smart_output("A string", "A string\n")
-        # Note: YAML uses b64 encoding for byte strings tagged by !!binary
-        self.assert_smart_output(
-            "High bytes: \xCA\xFE",
-            "!!binary |\n    SGlnaCBieXRlczogyv4=\n")
-        self.assert_smart_output(u"", "''\n")
-        self.assert_smart_output(
-            u"A unicode string (but really ascii)",
-            "A unicode string (but really ascii)\n")
-        # Any non-ascii Unicode will use UTF-8 encoding
-        self.assert_smart_output(u"中文", "\xe4\xb8\xad\xe6\x96\x87\n")
-        self.assert_smart_output({}, "{}\n")
+        self.assert_smart_output("", "")
+        self.assert_smart_output("A string", "A string")
+        self.assert_smart_output(
+            "High bytes: \xCA\xFE", "High bytes: \xca\xfe")
+        self.assert_smart_output("中文", "\xe4\xb8\xad\xe6\x96\x87")
         self.assert_smart_output(
             {u"public-address": u"ec2-1-2-3-4.compute-1.amazonaws.com",
              u"foo": u"bar",
              u"configured": True},
             ("configured: true\n"
              "foo: bar\n"
-             "public-address: ec2-1-2-3-4.compute-1.amazonaws.com\n"))
-        self.assert_smart_output(False, "false\n")
-        self.assert_smart_output(True, "true\n")
-        self.assert_smart_output(0.0, "0.0\n")
-        self.assert_smart_output(3.14159, "3.14159\n")
-        self.assert_smart_output(6.02214178e23, "6.02214178e+23\n")
-        self.assert_smart_output(0, "0\n")
-        self.assert_smart_output(42, "42\n")
+             "public-address: ec2-1-2-3-4.compute-1.amazonaws.com"))
 
=== modified file 'juju/hooks/tests/test_invoker.py'
--- juju/hooks/tests/test_invoker.py	2012-07-03 07:30:53 +0000
+++ juju/hooks/tests/test_invoker.py	2012-09-01 05:47:20 +0000
@@ -1,6 +1,7 @@
 # -*- encoding: utf-8 -*-
 
 from StringIO import StringIO
+import base64
 import json
 import logging
 import os
@@ -192,7 +193,7 @@
 
     def create_hook(self, hook, arguments):
         bin_path = self.get_cli_hook(hook)
-        fn = self.makeFile("#!/bin/sh\n'%s' %s" % (bin_path, arguments))
+        fn = self.makeFile("#!/bin/bash\n'%s' %s" % (bin_path, arguments))
         # make the hook executable
         os.chmod(fn, stat.S_IEXEC | stat.S_IREAD)
         return fn
@@ -1516,7 +1517,6 @@
         # we don't see units in the other container
         self.assertNotIn("mysql/0", self.log.getvalue())
 
-
     @defer.inlineCallbacks
     def test_open_and_close_ports(self):
         """Verify that port hook commands run and changes are immediate."""
@@ -1547,15 +1547,14 @@
             [{"port": 80, "proto": "tcp"},
              {"port": 53, "proto": "udp"}])
 
-
         result = yield exe(self.create_hook("close-port", "80/tcp"))
         self.assertEqual(result, 0)
         self.assertEqual(
             (yield unit_state.get_open_ports()),
-            [{"port": 53, "proto": "udp"} ,])
+            [{"port": 53, "proto": "udp"}])
         self.assertEqual(
             (yield container_state.get_open_ports()),
-            [{"port": 53, "proto": "udp"},])
+            [{"port": 53, "proto": "udp"}])
 
         yield exe.ended
         self.assertLogLines(
@@ -1658,6 +1657,16 @@
             "mysql", charm_name="mysql-format-v2")
         yield super(TestCharmFormatV2, self)._default_relations()
 
+    def make_zipped_file(self):
+        data_file = self.makeFile()
+        with open(data_file, "wb") as f:
+            # gzipped of 'abc' - however gzip will also includes the
+            # source file name, so easiest to keep it stable here as
+            # standard data
+            f.write("\x1f\x8b\x08\x08\xbb\x8bAP\x02\xfftmpr"
+                    "fyP0e\x00KLJ\x06\x00\xc2A$5\x03\x00\x00\x00")
+        return data_file
+
     @defer.inlineCallbacks
     def test_environment(self):
         """Ensure that an explicit setting of format: 2 works properly"""
@@ -1667,7 +1676,7 @@
         self.assertEqual(env["_JUJU_CHARM_FORMAT"], "2")
 
     @defer.inlineCallbacks
-    def test_output(self):
+    def test_smart_output(self):
         """Verify roundtripping"""
         hook_debug_log = capture_separate_log("hook", level=logging.DEBUG)
         hook_log = capture_separate_log("hook", level=logging.INFO)
@@ -1675,54 +1684,235 @@
             "database:42", "add", "mysql/0", self.relation,
             client_id="client_id")
 
-        # Byte strings are also supported by staying completely in
-        # YAML, so test that corner case. This also means users need
-        # to present valid YAML for any input:
-        data_file = self.makeFile(
-            yaml.safe_dump("But when I do drink, I prefer \xCA\xFE"))
+        # Test the support of raw strings, both from a file and from
+        # command line. Unicode can also be used - this is just
+        # rendered as UTF-8 in the shell; the source here is also
+        # UTF-8 - note it is not a Unicode string, it's a bytestring.
+        data_file = self.make_zipped_file()
         set_hook = self.create_hook(
             "relation-set",
-            "b=true i=42 f=1.23 s=ascii u=中文 d=@%s" % data_file)
+            "b=true f=1.23 i=42 s=ascii u=中文 d=@%s "
+            "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
+                data_file))
         yield exe(set_hook)
+
         result = yield exe(self.create_hook("relation-get", "- mysql/0"))
         self.assertEqual(result, 0)
 
-        # YAML guarantees that the keys will be sorted
-        # lexicographically; note that we output UTF-8 for Unicode
-        # when dumping YAML, so our source text (with this test file
-        # in UTF-8 itself) matches the output text, as seen in the
-        # characters for "zhongwen" (Chinese language).
+        # relation-get - uses YAML to dump keys. YAML guarantees that
+        # the keys will be sorted lexicographically; note that we
+        # output UTF-8 for Unicode when dumping YAML, so our source
+        # text (with this test file in UTF-8 itself) matches the
+        # output text, as seen in the characters for "zhongwen"
+        # (Chinese language).
         self.assertEqual(
             hook_log.getvalue(),
-            "b: true\n"
-            "d: !!binary |\n    QnV0IHdoZW4gSSBkbyBkcmluaywgSSBwcmVmZXIgyv4=\n"
-            "f: 1.23\n"
-            "i: 42\n"
+            "b: 'true'\n"
+            "d: !!binary |\n    H4sICLuLQVAC/3RtcHJmeVAwZQBLTEoGAMJBJDUDAAAA\n"
+            "f: '1.23'\n"
+            "i: '42'\n"
             "private-address: mysql-0.example.com\n"
+            "r: !!binary |\n    QnV0IHdoZW4gSSBkbyBkcmluaywgSSBwcmVmZXIgyv4=\n"
             "s: ascii\n"
-            "u: 中文\n\n")
+            "u: 中文\n")
 
-        # Log lines are not simply converted into Unicode, as in v1 format
+        # Note: backslashes are necessarily doubled here; r"XYZ"
+        # strings don't help with hexescapes
         self.assertLogLines(
             hook_debug_log.getvalue(),
             ["Flushed values for hook %r on 'database:42'" % (
                     os.path.basename(set_hook),),
-             "    Setting changed: 'b'=True (was unset)",
-             "    Setting changed: 'd'='But when I do drink, "
+             "    Setting changed: 'b'='true' (was unset)",
+             "    Setting changed: 'd'='\\x1f\\x8b\\x08\\x08\\xbb\\x8bAP\\x02"
+             "\\xfftmprfyP0e\\x00KLJ\\x06\\x00\\xc2A$5\\x03"
+             "\\x00\\x00\\x00' (was unset)",
+             "    Setting changed: 'f'='1.23' (was unset)",
+             "    Setting changed: 'i'='42' (was unset)",
+             "    Setting changed: 'r'='But when I do drink, "
              "I prefer \\xca\\xfe' (was unset)",
-             "    Setting changed: 'f'=1.23 (was unset)",
-             "    Setting changed: 'i'=42 (was unset)",
              "    Setting changed: 's'='ascii' (was unset)",
-             "    Setting changed: 'u'=u'\\u4e2d\\u6587' (was unset)"])
-
-        # Also ensure that invalid YAML is rejected; unlike earlier,
-        # this was not encoded with yaml.safe_dump
-        data_file = self.makeFile(
-            "But when I do drink, I prefer \xCA\xFE")
-        hook = self.create_hook("relation-set", "d=@%s" % data_file)
-        e = yield self.assertFailure(exe(hook), errors.CharmInvocationError)
-        self.assertEqual(str(e), "Error processing %r: exit code 1." % hook)
-        self.assertIn(
-            "yaml.reader.ReaderError: \'utf8\' codec can\'t decode byte #xca: "
-            "invalid continuation byte\n  in \"<string>\", position 30",
-            hook_log.getvalue())
+             "    Setting changed: 'u'=u'\\u4e2d\\u6587' (was unset)"
+             ])
+
+    @defer.inlineCallbacks
+    def test_exact_roundtrip_binary_data(self):
+        """Verify that binary data, including \x00, is roundtripped exactly"""
+        hook_log = capture_separate_log("hook", level=logging.INFO)
+        exe = yield self.ua.get_invoker(
+            "database:42", "add", "mysql/0", self.relation,
+            client_id="client_id")
+        data_file = self.make_zipped_file()
+
+        # relation-set can only read null bytes from a file; bash
+        # would otherwise silently drop
+        set_hook = self.create_hook("relation-set", "zipped=@%s" % (
+                data_file))
+        result = yield exe(set_hook)
+        self.assertEqual(result, 0)
+
+        # Abuse the create_hook method a little bit by adding a pipe
+        get_hook = self.create_hook("relation-get", "zipped mysql/0 | zcat")
+        result = yield exe(get_hook)
+        self.assertEqual(result, 0)
+
+        # Using the hook log for this verification does generate one
+        # extra \n (seen elsewhere in our tests), but this is just
+        # test noise: we are guaranteed roundtrip fidelity by using
+        # the picky tool that is zcat - no extraneous data accepted.
+        self.assertEqual(hook_log.getvalue(), "abc\n")
+
+    @defer.inlineCallbacks
+    def test_json_output(self):
+        """Verify roundtripping"""
+        hook_log = capture_separate_log("hook", level=logging.INFO)
+        exe = yield self.ua.get_invoker(
+            "database:42", "add", "mysql/0", self.relation,
+            client_id="client_id")
+
+        # Test the support of raw strings, both from a file and from
+        # command line. In addition, test Unicode indirectly by using
+        # UTF-8. Because the source of this file is marked as UTF-8,
+        # we can embed such characters directly in bytestrings, not
+        # just Unicode strings. This also works within the context of
+        # the shell.
+        raw = "But when I do drink, I prefer \xca\xfe"
+        data_file = self.makeFile(raw)
+        set_hook = self.create_hook(
+            "relation-set",
+            "b=true f=1.23 i=42 s=ascii u=中文 d=@%s "
+            "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
+                data_file,))
+        yield exe(set_hook)
+
+        result = yield exe(self.create_hook(
+                "relation-get", "--format=json - mysql/0"))
+        self.assertEqual(result, 0)
+
+        # YAML serialization internally has converted (transparently)
+        # UTF-8 to Unicode, which can be rendered by JSON. However the
+        # "cafe" bytestring is invalid JSON, so verify that it's been
+        # Base64 encoded.
+        encoded = base64.b64encode(raw)
+        self.assertEqual(
+            hook_log.getvalue(),
+            '{"b": "true", '
+            '"d": "%s", '
+            '"f": "1.23", '
+            '"i": "42", '
+            '"private-address": "mysql-0.example.com", '
+            '"s": "ascii", '
+            '"r": "%s", '
+            '"u": "\\u4e2d\\u6587"}\n' % (encoded, encoded))
+
+    @defer.inlineCallbacks
+    def common_relation_set(self):
+        hook_log = capture_separate_log("hook", level=logging.INFO)
+        exe = yield self.ua.get_invoker(
+            "database:42", "add", "mysql/0",
+            self.relation, client_id="client_id")
+        raw = "But when I do drink, I prefer \xCA\xFE"
+        data_file = self.makeFile(raw)
+        set_hook = self.create_hook(
+            "relation-set",
+            "s='some text' u=中文 d=@%s "
+            "r=\"$(echo -en 'But when I do drink, I prefer \\xCA\\xFE')\"" % (
+                data_file))
+        result = yield exe(set_hook)
+        self.assertEqual(result, 0)
+        defer.returnValue((exe, hook_log))
+
+    @defer.inlineCallbacks
+    def test_relation_get_ascii(self):
+        """Verify that ascii data is roundtripped"""
+        exe, hook_log = yield self.common_relation_set()
+        result = yield exe(self.create_hook("relation-get", "s mysql/0"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "some text\n")
+
+    @defer.inlineCallbacks
+    def test_relation_get_raw(self):
+        """Verify that raw data is roundtripped"""
+        exe, hook_log = yield self.common_relation_set()
+        result = yield exe(self.create_hook("relation-get", "r mysql/0"))
+        self.assertEqual(result, 0)
+        self.assertEqual(
+            hook_log.getvalue(), "But when I do drink, I prefer \xca\xfe\n")
+
+    @defer.inlineCallbacks
+    def test_relation_get_unicode(self):
+        """Verify Unicode is roundtripped (via UTF-8) through the shell"""
+        exe, hook_log = yield self.common_relation_set()
+
+        result = yield exe(self.create_hook("relation-get", "u mysql/0"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "中文\n")
+
+    @defer.inlineCallbacks
+    def setup_config(self):
+        hook_log = self.capture_logging("hook")
+        exe = yield self.ua.get_invoker(
+            "db:42", "add", "mysql/0", self.relation, client_id="client_id")
+        context = yield self.ua.get_context("client_id")
+        config = yield context.get_config()
+        with open(self.make_zipped_file(), "rb") as f:
+            data = f.read()
+        config.update({
+            "b": True,
+            "f": 1.23,
+            "i": 42,
+            "s": "some text",
+            # uses UTF-8 encoding in this test script
+            "u": "中文",
+            # use high byte and null byte characters
+            "r": data
+            })
+        yield config.write()
+        defer.returnValue((exe, hook_log))
+
+    @defer.inlineCallbacks
+    def test_config_get_boolean(self):
+        """Validate that config-get returns lowercase names of booleans."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "b"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "true\n")
+
+    @defer.inlineCallbacks
+    def test_config_get_float(self):
+        """Validate that config-get returns floats without quotes."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "f"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "1.23\n")
+
+    @defer.inlineCallbacks
+    def test_config_get_int(self):
+        """Validate that config-get returns ints without quotes."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "i"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "42\n")
+
+    @defer.inlineCallbacks
+    def test_config_get_ascii(self):
+        """Validate that config-get returns ascii strings."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "s"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "some text\n")
+
+    @defer.inlineCallbacks
+    def test_config_get_raw(self):
+        """Validate config-get can work with high and null bytes."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "r | zcat"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "abc\n")
+
+    @defer.inlineCallbacks
+    def test_config_get_unicode(self):
+        """Validate that config-get returns raw strings containing UTF-8."""
+        exe, hook_log = yield self.setup_config()
+        result = yield exe(self.create_hook("config-get", "u"))
+        self.assertEqual(result, 0)
+        self.assertEqual(hook_log.getvalue(), "中文\n")
 
=== modified file 'juju/lib/format.py'
--- juju/lib/format.py	2012-06-22 19:08:23 +0000
+++ juju/lib/format.py	2012-09-01 05:47:20 +0000
@@ -42,6 +42,14 @@
 
         return data
 
+    def _parse_value(self, key, value):
+        """Interprets value as a str"""
+        return value
+
+    def should_delete(self, value):
+        """Whether `value` implies corresponding key should be deleted"""
+        return not value.strip()
+
 
 class PythonFormat(BaseFormat):
     """Supports backwards compatibility through str and JSON encoding."""
@@ -52,9 +60,9 @@
         """Formats `data` using Python str encoding"""
         return str(data)
 
-    def _parse_value(self, key, value):
-        """Interprets value as a str"""
-        return value
+    def format_raw(self, data):
+        """Add extra \n seen in Python format, so not truly raw"""
+        return self.format(data) + "\n"
 
     # For the old format: 1, using JSON serialization introduces some
     # subtle issues around Unicode conversion that then later results
@@ -69,13 +77,6 @@
         """Loads data, but also converts str to Unicode"""
         return json.loads(data)
 
-    def should_delete(self, value):
-        """Whether `value` implies corresponding key should be deleted"""
-        # In format: 1, all values are strings, but possibly with
-        # spaces. The strip reduces strings consisting only of spaces,
-        # or otherwise empty, to an empty string.
-        return not value.strip()
-
 
 class YAMLFormat(BaseFormat):
     """New format that uses YAML, so no unexpected encoding issues"""
@@ -98,13 +99,12 @@
         # Also remove any extra \n, will still be valid yaml
         return serialized.rstrip("\n")
 
-    def _parse_value(self, key, value):
-        # Ensure roundtripping to/from yaml if format: 2; in
-        # particular this ensures that true/false work as desired
-        try:
-            return yaml.safe_load(value)
-        except Exception:
-            raise JujuError("Invalid YAML value (argument:%s)" % key)
+    def format_raw(self, data):
+        """Formats `data` as a raw string if str, otherwise as YAML"""
+        if isinstance(data, str):
+            return data
+        else:
+            return self.format(data)
 
     # Use the same format for dump
     dump = format
@@ -113,13 +113,6 @@
         """Loads data safely, ensuring no Python specific type info leaks"""
         return yaml.safe_load(data)
 
-    def should_delete(self, value):
-        """Whether `value` implies corresponding key should be deleted"""
-        # In format: 2, values were already parsed by yaml.safe_load;
-        # in particular, a string consisting only of whitespace is
-        # parsed as None.
-        return value is None
-
 
 def is_valid_charm_format(charm_format):
     """True if `charm_format` is a valid format"""
 
=== modified file 'juju/lib/tests/test_format.py'
--- juju/lib/tests/test_format.py	2012-06-22 19:08:23 +0000
+++ juju/lib/tests/test_format.py	2012-09-01 05:47:20 +0000
@@ -223,7 +223,7 @@
     def assert_parse(self, data):
         """Verify input parses as expected, including from a data file"""
         formatter = YAMLFormat()
-        formatted = formatter.format(data)
+        formatted = formatter.format_raw(data)
         data_file = self.makeFile(formatted)
         kvs = ["formatted=%s" % formatted,
                "file=@%s" % data_file]
@@ -234,27 +234,9 @@
     def test_parse_keyvalue_pairs(self):
         """Verify key value pairs parse for a wide range of YAML inputs."""
         formatter = YAMLFormat()
-        self.assert_parse(None)
         self.assert_parse("")
         self.assert_parse("A string")
         self.assert_parse("High bytes: \xCA\xFE")
-        self.assert_parse(u"")
-        self.assert_parse(u"A unicode string (but really ascii)")
-        self.assert_parse(u"中文")
-        self.assert_parse({})
-        self.assert_parse(
-            {u"public-address": u"ec2-1-2-3-4.compute-1.amazonaws.com",
-             u"foo": u"bar",
-             u"configured": True})
-        self.assert_parse([])
-        self.assert_parse(["abc", "xyz", 42, True])
-        self.assert_parse(False)
-        self.assert_parse(True)
-        self.assert_parse(0.0)
-        self.assert_parse(3.14159)
-        self.assert_parse(6.02214178e23)
-        self.assert_parse(0)
-        self.assert_parse(42)
 
         # Raises an error if no such file
         e = self.assertRaises(
@@ -271,14 +253,6 @@
         self.assertEquals(
             str(e), "Expected `option=value`. Found `foobar`")
 
-        # Raises an error if the value is invalid YAML
-        e = self.assertRaises(
-            JujuError,
-            formatter.parse_keyvalue_pairs, ["content=\xCA\FE"])
-        self.assertEquals(
-            str(e),
-            "Invalid YAML value (argument:content)")
-
     def assert_dump_load(self, data, expected):
         """Asserts expected formatting and roundtrip through dump/load"""
         formatter = YAMLFormat()
@@ -320,11 +294,15 @@
         self.assert_dump_load(42, "data: 42")
 
     def test_should_delete(self):
-        """Verify only `None` values (as YAML loaded) indicate deletion"""
-        formatter = YAMLFormat()
+        """Verify empty or whitespace only strings indicate deletion"""
+        formatter = PythonFormat()
         self.assertFalse(formatter.should_delete("0"))
         self.assertFalse(formatter.should_delete("something"))
-        self.assertFalse(formatter.should_delete(""))
-        self.assertFalse(formatter.should_delete("   "))
-        self.assertFalse(formatter.should_delete(0))
-        self.assertTrue(formatter.should_delete(None))
+        self.assertTrue(formatter.should_delete(""))
+        self.assertTrue(formatter.should_delete("   "))
+
+        # Verify that format: 1 can only work with str values
+        e = self.assertRaises(AttributeError, formatter.should_delete, 42)
+        self.assertEqual(str(e), "'int' object has no attribute 'strip'")
+        e = self.assertRaises(AttributeError, formatter.should_delete, None)
+        self.assertEqual(str(e), "'NoneType' object has no attribute 'strip'")