On Tue, Apr 10, 2018 at 1:32 PM, Sage Weil <sweil@xxxxxxxxxx> wrote: > 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>>>". Oh, right -- yeah, that's a clear improvement on the current situation. > 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. If we put lists in here then it'll break anyone who's currently parsing this output and expecting strings -- I'd be more inclined to do the "unprintable blob" thing and then go ahead and deprecate it, as we know that nobody can be relying on this command to get binary strings out today. John > 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
