On Tue, 10 Apr 2018, John Spray wrote: > On Tue, Apr 10, 2018 at 4:39 AM, Sage Weil <sweil@xxxxxxxxxx> wrote: > > http://tracker.ceph.com/issues/23622 is a test failure because 'ceph > > config-key dump' is not spitting out valid JSON. That's because it looks > > like this: > > > > { > > "config-history/1/": "`Zd\u0008\u0000\u0000\u0000\u0000", > > "config-history/2/": "`Z4w*\u001b\u0000\u0000\u0000\u0000", > > "config-history/2/+global/mon_pg_warn_min_per_osd": "3", > > "config-history/2/+global/osd_pool_default_min_size": "1", > > "config-history/2/+global/osd_pool_default_size": "1", > > ... > > > > Those first two keys probably didn't paste into my message properly, but > > they're binary data that aren't valid utf8 or unicode... just random > > bytes. I didn't realize this before, but it seems that JSON isn't > > actually capable of representing arbitrary binary strings... only > > printable utf8/unicode stuff. > > > > In contrast, config-key will happily store binary data. I think that > > currently nothing (in tree) does that except for the new encoded blob for > > each config-history change entry (above). But even so, storing binary > > data seems useful. But then 'ceph config-key dump' can't actually print > > it in JSON. > > > > Should we > > > > 1- Print something else there even though it is not strictly correct? > > That would mean that something can't parse the (JSON) dump output and > > get the right answer. > > Definitely not this: if we were going to output something JSON-like > but not quite JSON, we'd be better off making a bigger jump to another > binary format. I meant something that is still valid JSON but not the correct string. Like "<<<unprintable blob of length x>>>". But what about 6- Instead of a string, print an array of characters. e.g., { "foo": "abc123", "bar": [ 1, 243, 23, 0, 0, 23, 433 ], "baz": "another printable string" } It makes the output a complete and unambiguous representation of the state, albeit on that is a bit less friendly to a human consumer? We could still deprecated it. sage > > > 2- Disallow binary data. This seems weak, but at least behavior would be > > consistent. We'll probably break some random out-of-tree users out there. > > We could allow binary data in set operations, but exclude it from the > config-key dump output, users who cared would have to go get it with > individual commands. > > > 3- Use XML as the output format for this command? > > Yuck! (and as Ilya says, XML has its own issues) > > > 4- Remove the 'dump' command entirely and make you get each item. There > > is still 'ceph config-key list' to get the keys you would need to > > individually get. > > This isn't so unreasonable: since values are potentially large > (especially if there are binary blobs in there!), dump is always at > risk of giving unhelpfully oversized output. It's also going to be of > limited human-readable interest the more we use it from mgr modules > for things like JSON blobs of state. > > So maybe deprecate dump, and also make dump omit values that are unprintable? > > > All of these options kind of suck. Is there a better option I'm missing? > > > I'm feeling pretty let down by the collective industry decision to use > > JSON for all the things. > > Yeah... lots of people preferring grpc to JSON-over-whatever these days. > > John > > > > > :( sage > > -- > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
