On Wed, 6 Feb 2013, Yehuda Sadeh wrote:
> On Wed, Feb 6, 2013 at 2:23 PM, Sage Weil <sage@xxxxxxxxxxx> wrote:
> > On Wed, 6 Feb 2013, Yehuda Sadeh wrote:
> >> The matter of json decoding was brought up as part of the discussion
> >> of the ceph management api. I recently had to deal with it with the
> >> rados gateway, as I wasn't too happy was the current mechanisms that
> >> were used for this purpose. So I introduced a new decode_json()
> >> functionality (now in common/ceph_json.* at the wip-json-decode
> >> branch). It's very similar to the regular encode/decode stuff that we
> >> use to encode/decode structures. Encoding is being done using the
> >> already existing dump() method, and decoding will be done through the
> >> new decode_json() method.
> >>
> >> As an example we'll define a new UserInfo structure:
> >>
> >> struct UserInfo {
> >> string uid;
> >> string display_name;
> >> int max_buckets;
> >> list<Key> keys;
> >>
> >> void dump(Formatter *f) const;
> >> void decode_json(JSONObj *obj);
> >> };
> >>
> >> The dump() method will look like this:
> >>
> >> void UserInfo::dump(Formatter *f) const
> >> {
> >> f->open_object_section("user_info");
> >> f->dump_string("user_id", uid);
> >> f->dump_string("display_name", display_name);
> >> f->dump_int("max_buckets", (int)max_buckets);
> >>
> >> f->open_array_section("keys");
> >> for (list<Key>::iterator iter = keys.begin(); siter != keys.end(); ++iter) {
> >> iter->dump(f);
> >> }
> >> f->close_section();
> >>
> >> f->close_section();
> >> }
> >>
> >> It is recommended that every dump() method opens an object section and
> >> puts everything within, so that every object could be easily embedded
> >> within other objects. We can also probably create some template that
> >
> > The current loose convention is that the dump objects do not define the
> > container they dump into. It's a bit weird, but it lets you do things
> > like:
> >
> > map<Key, Value> foo;
> >
> > f->open_array_section("keys");
> > for (map<Key,Value>::iterator iter = keys.begin(); siter != keys.end(); ++iter) {
> > f->open_object_section("thing");
> > f->dump_string("key", iter->first);
> > iter->second.dump(f);
> > f->close_section();
> > }
> > f->close_section();
> >
> > Admittedly that doesn't come up *that* often, but it happened enough when
> > we first started doing the dump stuff that all/most of the dumpers work
> > that way.
> >
> > How should we deal with that?
> >
>
> Modify the dumpers to not work this way? It's a matter of conventions,
> not set in stone. If we keep this form then we'd need to create
> decoders for these records:
>
> struct KeyRecord {
> Key k;
> Value v;
>
> void decode_json(JSONObj *obj) {
> JSONDecoder::decode_json("key", key, obj);
> v.decode_json(obj);
> }
> };
>
> This will work, assuming Value doesn't have any field indexed by "key".
I think it mostly comes up in dumping other structures, not parsing, so it
may be mostly ok on that end. I'm not sure how to manage dumping, though,
if the section is opened inside the value object's dump().
This also worries me: ceph_dencoder does
jf.open_object_section("object");
den->dump(&jf);
jf.close_section();
which means that this sort of change will probably the type
encoding/decode unit tests we run.
Maybe something like
void encode_json(Formatter *f) const
{
f->open_object_section("mytype");
dump(f);
f->close_section();
}
will give us the cleaner interface you're looking for. And the
ceph-dencoder type definitions can mark types which implement one or the
other... or we can implement the encode_json one for all objects as a
wrapper via a macro or something we don't have to duplicate teh
boilerplate everywhere?
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
