+1 for No type / Non native types only / All types. +1 for stamping out a new version of GraphSON (though i forget what i had planned to allow that to work)
On Thu, Apr 21, 2016 at 10:56 AM, Kevin Gallardo <gallardo.kev...@gmail.com> wrote: > I agree this change would imply to stamp out a new version of GraphSON > because it breaks compatibility for types embedded anyway, I don't see how > to workaround that.. Having an option on the current version to make both > compatible would be very prone to errors, might as well mark a clear > evolution with a new version, what do you think? > > I think it would be worth having only 3 settings : No type / Non native > types only / All types. Looks more straightforward imo. > > Cheers. > > On 2016-04-11 18:04, Stephen Mallette <s...@gmail.com> wrote: > > It would be nice to get to some consensus on this thread so we could > put> > > something out for review on Gremlin Users and ultimately VOTE. I've> > > gathered from this thread that:> > > > > 1. It seems like there is some consensus on typing JSON non-natives > only> > > (as opposed to everything). Embedding a type for existing JSON types > would> > > be more consistent, but it seems redundant. I suppose we could hang > that> > > out there for public opinion on the gremlin-users list as i'm not sure > what> > > I would like here. I'm personally not a consumer of GraphSON with > embedded> > > types (especially off the jvm) so I'd rely on the drivers devs and the> > > greater user community to guide the vote i cast there.> > > 2. The general structure of the revised GraphSON that Kevin proposed > seems> > > to be generally accepted and I think using the non-canonical name for > the> > > types makes sense, though I suppose we leave open the chance for > collision> > > under that model. Perhaps there is a configuration we could add there.> > > 3. Dylan wondered: "It still feels a little too half hearted to warrant > a> > > new serializer version though?" I don't see how we can't stamp out a > new> > > version of GraphSON if we do this. The format for embedded types is> > > changing (untyped will stay the same). Unless there is a way to make > this a> > > configuration of the current version so that it can easily read both> > > formats? maybe this is all just configuration of the same GraphSON > format?> > > you have the following configuration options:> > > > > a. JSON native> > > b. full embedded type with canonical name> > > c. full embedded type with simple name> > > b. partial embedded type with canonical name> > > c. partial embedded type with simple name> > > > > ....hmm even with those options you break version because both b and c> > > options (which would be represent complete non-lossy approach wouldn't> > > match the format we have now (as the current format doesn't type json> > > primitives) anyway, open to ideas on how we could do this without having > to> > > version GraphSON....> > > > > > > > > On Sat, Apr 9, 2016 at 5:04 PM, Dylan Millikin <dy...@gmail.com>> > > wrote:> > > > > > The double map comment if I recall correctly was in the case of > typing> > > > everything (even JSON natives). So typing a map would be the actual > case.> > > >> > > > At this point I think it would be good to make sure whichever model we > pick> > > > prevents as much type loss as possible before moving forward. > Consistency +> > > > Java classes + JSON non-natives is definitely a convenience but we > might as> > > > well go the extra mile if we're going to add a v2 serializer (if - as > it> > > > should go through a DISCUSS/VOTE).> > > >> > > > On Thu, Apr 7, 2016 at 3:57 PM, Kevin Gallardo <ga...@gmail.com>> > > > wrote:> > > >> > > > > > - List, like suggested in the original thread. Pros: simple. Cons: > if> > > > > the>> > > > > > generate List has two maps your un-serializer will need to look > ahead> > > > > into>> > > > > > the maps to see if one has a "@class" key as we can't guaranty > List> > > > > order.>> > > > >> > > > > Sorry but I don't see, with the model proposed, how can the List > contain> > > > 2> > > > > maps with one being the map with the type, I can't manage to find > an> > > > > example, but if you have one in mind, please let me know. Moreover, > why> > > > > can't we guaranty the order? GraphSON serializers methods explicitly > call> > > > > the type serializers before serializing the values, and since it is > a> > > > list,> > > > > the order is conserved.> > > > > I am not against the Map approach, but I don't see a valid reason > not to> > > > > use the list, and it seems less verbose..> > > > >> > > > > I agree with your conclusion, seems like a reasonable decision for > Java> > > > > classes + JSON non-natives. Just to clarify, you mention overhead > for the> > > > > server serializers/GraphSONWriter with Generic names, I am not sure > it> > > > > applies though, the intention originally would be to encode only > the> > > > Class> > > > > name rather than the canonical name. i.e. for example from> > > > > "java.time.Instant" to "Instant", and this does not imply overhead > on the> > > > > serialization process.> > > > >> > > > > > > I'd ask for the same question concerning typing on natively > supported> > > > > JSON types, is the overhead worth it?> > > > > > I think that there have been a number of struggles for non-jvm> > > > languages> > > > > around discerning float/double and int/long with respect to > graphson.> > > > >> > > > > Indeed, with this proposal, since the typing would work both ways> > > > > (ser/de), the GraphSONReader would be able to coerce automatically > an> > > > > Element having non-JSON-native properties into the correct type, > from> > > > > reading the correctly typed JSON.> > > > >> > > > > Thanks for your answer.> > > > >> > > > > On 2016-04-05 16:03, Dylan Millikin <d....@gmail.com> wrote:> > > > > > At this point I'm more or less neutral here. As a recap, I guess > there> > > > > are>> > > > > > two points:>> > > > > >> > > > > > 1) Making the graphSON typing more homogenous across types (Map vs > List> > > > > vs>> > > > > > Long), no special cases. Here the values could either be > encapsulated> > > > in> > > > > :>> > > > > > - List, like suggested in the original thread. Pros: simple. Cons: > if> > > > > the>> > > > > > generate List has two maps your un-serializer will need to look > ahead> > > > > into>> > > > > > the maps to see if one has a "@class" key as we can't guaranty > List> > > > > order.>> > > > > > It doesn't imply much overhead though.>> > > > > > - Map. Pros: more thorough / theoretically a tad faster on the > client> > > > > end.>> > > > > > Cons: more verbose, less readable.>> > > > > >> > > > > > 2) What to type and how to type it (specific Java class vs > generic> > > > > naming,>> > > > > > etc.),>> > > > > >> > > > > > How to print the types:>> > > > > >> > > > > > - As java classes, Pros: Are self explanatory, native to the> > > > serializing>> > > > > > end so no overhead. Cons: implies systematic overhead on the > client end> > > > > for>> > > > > > non-jvm mapping.>> > > > > > - As generic names, Pros: non-jvm mapping can cherry pick which > types> > > > it>> > > > > > needs to map so less overhead. Cons: specific documentation is> > > > required,> > > > > i>> > > > > > guess that theoretically there's more overhead for the server > end>> > > > > > serializer as well.>> > > > > >> > > > > > What to type:>> > > > > >> > > > > > - Everything. Pros: if added to 1) it would make GraphSON very> > > > > homogenous>> > > > > > and fail proof. Cons: overhead overhead overhead (need to figure > out> > > > if>> > > > > > it's significant or not)>> > > > > > - JSON non-natives only. Pros: less overhead, simplifies the > current>> > > > > > implementation. Cons: potentially not as safe. Though I suspect > that> > > > any>> > > > > > language supporting JSON should have workarounds for some of the> > > > > pitfalls.>> > > > > > So probably a non-issue. Doesn't help in solving any of the > int/long> > > > and>> > > > > > float/double issues though.>> > > > > >> > > > > >> > > > > > Conclusion:>> > > > > >> > > > > > Looking at the above I guess that implementing 1) (either choice) > and> > > > 2)>> > > > > > Java classes + JSON non-natives. Provides a simpler and easier to > use>> > > > > > GraphSON representation with equal usability to the current> > > > > implementation.>> > > > > > It still feels a little too half hearted to warrant a new > serializer>> > > > > > version though?>> > > > > >> > > > > > On Tue, Apr 5, 2016 at 4:07 PM, Stephen Mallette <sp...@gmail.com>>> > > > > > > wrote:>> > > > > >> > > > > > > > I'd ask for the same question concerning typing on natively> > > > > supported>> > > > > > > JSON types, is the overhead worth it?>> > > > > > >>> > > > > > > I think that there have been a number of struggles for non-jvm> > > > > languages>> > > > > > > around discerning float/double and int/long with respect to> > > > graphson.>> > > > > > > That's been true of TinkerPop 2.x and 3.x. We did a better job > in> > > > 3.x> > > > > by>> > > > > > > pushing some of the responsibility for id parsing (for graphs > that> > > > use>> > > > > > > numeric types for ids) down to the Graph implementation level so > that> > > > > has>> > > > > > > been helpful, but there's nothing for situations like:>> > > > > > >>> > > > > > > v.property("someFloatField", x)>> > > > > > >>> > > > > > > we typically recommend that users coerce values as:>> > > > > > >>> > > > > > > v.property("someFloatField", Float.valueOf(x))>> > > > > > >>> > > > > > > or just don't use float in your schema. I'm not saying the > overhead> > > > > of>> > > > > > > this is worth what we'd get, but just pointing out that there is > a> > > > > commonly>> > > > > > > seen problem to solve here. It might be fine to stick with our> > > > current>> > > > > > > advice on implementations patterns for this. We might want to > raise> > > > > this>> > > > > > > issue on gremlin-users for more feedback once we have better> > > > consensus>> > > > > > > here.>> > > > > > >>> > > > > > >>> > > > > > >>> > > > > > > On Mon, Apr 4, 2016 at 12:03 PM, Kevin Gallardo <>> > > > > > > kevin.galla...@datastax.com>> > > > > > > > wrote:>> > > > > > >>> > > > > > > > Thank you for your answers.>> > > > > > > >>> > > > > > > > I agree with the reasoning of needing the full types names > when it> > > > > comes>> > > > > > > > to check the specification, however after you saw the specs > and you> > > > > know>> > > > > > > > how to deserialize it, is it worth it to still get the full > types> > > > in> > > > > all>> > > > > > > > the server's responses afterwards?>> > > > > > > >>> > > > > > > > I'd ask for the same question concerning typing on natively> > > > > supported>> > > > > > > JSON>> > > > > > > > types, is the overhead worth it? If you only want to be able > to>> > > > > > > distinguish>> > > > > > > > non native types from the native ones.>> > > > > > > > It could also maybe be possible to have some kinds of "level" > for> > > > > typing>> > > > > > > > then, "no type", "only custom types", "all types". What do > you> > > > > think? (I>> > > > > > > am>> > > > > > > > not sure how typing native JSON elements is supported by > Jackson> > > > > though).>> > > > > > > >>> > > > > > > > Cheers.>> > > > > > > >>> > > > > > > > On 2016-04-01 14:56, Dylan Millikin <d....@gmail.com> > wrote:>> > > > > > > > > Ok I see. That might be a little redundant but it definitely > has> > > > > it's>>> > > > > > > > > advantages.>>> > > > > > > > > My initial thought was mostly like Kevin's regarding JSON > native> > > > > types.>> > > > > > > > I>>> > > > > > > > > had a few reservations regarding the type naming and I've > given> > > > > this a>> > > > > > > > bit>>> > > > > > > > > more thought :>>> > > > > > > > >>> > > > > > > > > I think the java specific typing is a good thing because it> > > > > directly>>> > > > > > > > > references those types as being java types (with those > specs).> > > > > Typing>> > > > > > > in>>> > > > > > > > > other languages do not follow any standard, "int" can mean> > > > > anything>> > > > > > > > ranging>>> > > > > > > > > from "short" to "long" to nothing depending on your language > and>> > > > > > > system.>>> > > > > > > > > PHP is horrible for this, int = int on 32b systems and int = > long> > > > > on>> > > > > > > 64b>>> > > > > > > > > systems, and I believe other languages also have similar > issues> > > > > (the>> > > > > > > > Ariane>>> > > > > > > > > space program notoriously lost one of their rockets to this > kind> > > > > of>> > > > > > > > typing>>> > > > > > > > > issue). So drivers/clients -should- map the types anyways > and> > > > the>> > > > > > > > current>>> > > > > > > > > implementation makes it easy to understand where to go for> > > > specs.>>> > > > > > > > >>> > > > > > > > > If anything, considering the above, you may actually want to > type> > > > > JSON>>> > > > > > > > > native types as well to make this completely fail-proof.>>> > > > > > > > >>> > > > > > > > > This goes in the completely opposite direction of what the > OP>> > > > > > > suggested>>> > > > > > > > > though. Thoughts?>>> > > > > > > > >>> > > > > > > > >>> > > > > > > > > On Fri, Apr 1, 2016 at 3:17 PM, Stephen Mallette <> > > > sp...@gmail.com> > > > > >>>> > > > > > > > > wrote:>>> > > > > > > > >>> > > > > > > > > > I haven't yet had a chance to fully think this change > through,> > > > > but I>> > > > > > > > think>>> > > > > > > > > > the idea would be to produce a new version of GraphSON and > keep> > > > > the>> > > > > > > > old>>> > > > > > > > > > version around for backward compatibility. It would also > only> > > > be> > > > > a>> > > > > > > > breaking>>> > > > > > > > > > change for type embedded GraphSON, which i would guess > the> > > > > minority>> > > > > > > > are>>> > > > > > > > > > using right now as its verbosity and dependence on java > types> > > > > makes>> > > > > > > it>>> > > > > > > > > > borderline unusable off the JVM. I think this is a > nice> > > > > proposal>> > > > > > > > because>>> > > > > > > > > > it tries to make embedded types something that could > actually> > > > be>> > > > > > > > usable in>>> > > > > > > > > > a more general fashion across different programming > languages.>> > > > > > > Anyway,>> > > > > > > > the>>> > > > > > > > > > key point is that we would want to try to still support > the> > > > old>> > > > > > > > version of>>> > > > > > > > > > type-embedded graphson in addition to what kevin has> > > > proposed.>>> > > > > > > > > >>>> > > > > > > > > > On Fri, Apr 1, 2016 at 9:11 AM, Dylan Millikin <> > > > dy...@gmail.com> > > > > >>>> > > > > > > > > > wrote:>>> > > > > > > > > >>>> > > > > > > > > > > I also thought about this when I tried to set this > serializer> > > > > up>> > > > > > > > with the>>> > > > > > > > > > > php driver. This is quite a breaking change though.>>> > > > > > > > > > >>>> > > > > > > > > > > On Thu, Mar 31, 2016 at 1:33 PM, Kevin Gallardo <>>> > > > > > > > > > > kevin.galla...@datastax.com>>> > > > > > > > > > > > wrote:>>> > > > > > > > > > >>>> > > > > > > > > > > > Hi,>>> > > > > > > > > > > >>>> > > > > > > > > > > >>>> > > > > > > > > > > > By working closely with Tinkerpop I have been > interacting> > > > > quite a>> > > > > > > > bit>>> > > > > > > > > > > with>>> > > > > > > > > > > > the GraphSON format and I have come up with the need > of> > > > > using>> > > > > > > the>>> > > > > > > > > > > > "embedTypes" option of the GraphSON Mapper to get > more> > > > > insight of>> > > > > > > > the>>> > > > > > > > > > > types>>> > > > > > > > > > > > of the data I was transferring through the GraphSON> > > > > payload.>>> > > > > > > > > > > >>>> > > > > > > > > > > > By default vertices, edges, having properties that > have> > > > > another>> > > > > > > > type>>> > > > > > > > > > than>>> > > > > > > > > > > > the natively supported types in the JSON format are > encoded> > > > > as>> > > > > > > > their>>> > > > > > > > > > > String>>> > > > > > > > > > > > representation. Meaning that a Vertex property being a > UUID> > > > > will>> > > > > > > > look>>> > > > > > > > > > > like>>> > > > > > > > > > > > "24B7DED2-F72A-11E5-815D-79DF61FECC63" which, for the > user>> > > > > > > > consuming>>> > > > > > > > > > the>>> > > > > > > > > > > > JSON produced by the GraphSONWriter, will not be> > > > > distinguishable>> > > > > > > > from>>> > > > > > > > > > > being>>> > > > > > > > > > > > a String or a UUID. That is usually why you need to> > > > activate> > > > > the>> > > > > > > > type>>> > > > > > > > > > > > embedding of GraphSON. Documentation about the > current> > > > > output of>>> > > > > > > > > > GraphSON>>> > > > > > > > > > > > typing :>>> > > > > > > > > > > >>>> > > > > > > > > > > >>>> > > > > > > > > > >>>> > > > > > > > > >>> > > > > > > >>> > > > > > >> > > > >> > > > > http://tinkerpop.apache.org/docs/3.1.1-incubating/reference/#graphson-reader-writer> > > > > > >> > > > > > > > >>> > > > > > > > > > > >>>> > > > > > > > > > > > However, right now, embedding types with GraphSON > comes> > > > with> > > > > a>> > > > > > > few>>> > > > > > > > > > trade>>> > > > > > > > > > > > offs :>>> > > > > > > > > > > >>>> > > > > > > > > > > > - > [message truncated...] > > > — > Kevin Gallardo > kgdo.me > >