[GitHub] [tinkerpop] spmallette opened pull request #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["spmallette (GitHub)" <gi...@apache.org>]

https://issues.apache.org/jira/browse/TINKERPOP-1435

Implemented `Duration`, `Char`, `ByteBuffer`, `Byte`, `BigInteger` and `BigDecimal` which were the most critical ones right now. Tried to do `InetAddress` but there didn't seem to be anything in Python suitable to handled that type except for `ip_address` but it wouldn't parse things like "localhost" and other things that Java will so it seemed a bit incompatible. Also ignored all the `java.time.*` things for now...wonder if those shouldn't just be deprecated....



[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on issue #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["spmallette (GitHub)" <gi...@apache.org>]

The server won't reject `g:Int32` if it's a byte size value but it won't interpret it as a java `Byte`. This change just allows python to interpret/send the type. I'm not sure what is a breaking change here though. Right now, if Gremlin Server sends a `gx;Byte` python can't deserialize it and gremlin-python has no way to send a single `gx:Byte`.  isn't this an additive feature?  

I suppose that maybe BigInteger might produce different behavior that could be considered breaking. Currently, `gx:BigInteger` coming from Gremlin Server won't be deserialized and I imagine results in error and if python has a number outside of 64bit size and sends it as `g:Long` it will fail to deserialize on Gremlin Server. So, behavior of numbers is a little different because they remove error conditions, but isn't that ok? 

or am i not thinking about something correctly? (entirely possibly that i am actually as i might be misunderstanding how python is handling some of this stuff given my middling knowledge of the language)

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] aboudreault commented on issue #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["aboudreault (GitHub)" <gi...@apache.org>]

It is. But not sure if we can do otherwise if we want to add this serializer. Stephen, is the server really rejecting the data if we send a gx:Int32 in the range of the Byte possible values?

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette closed pull request #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["spmallette (GitHub)" <gi...@apache.org>]

[ pull request closed by spmallette ]

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on pull request #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["jorgebay (GitHub)" <gi...@apache.org>]

Shouldn't we throw in case we exceed the range of a int64?

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on issue #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["jorgebay (GitHub)" <gi...@apache.org>]

Just wanted to double check, thanks @spmallette and @aboudreault .

This is a huge improvement, VOTE +1

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] jorgebay commented on issue #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["jorgebay (GitHub)" <gi...@apache.org>]

It's looking good, I have a doubt: would addition of `ByteIO` and others represent a breaking change for Python users? I mean the expected type was x and now is y.

cc @aboudreault 

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] aboudreault commented on issue #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["aboudreault (GitHub)" <gi...@apache.org>]

oh, right.. at the moment the user will get back a non-deserialized `{''@type': 'gx:Byte', '@value': 42}`. I had in mind that the `int` would be returned as is.

I think we are good Stephen.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on pull request #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["spmallette (GitHub)" <gi...@apache.org>]

in python `long` is not limited to 64 bits so i thought it would be a better translation to magically convert to `BigInteger` so that java could consume it.  not good?

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] aboudreault commented on pull request #1060: TINKERPOP-1435 GraphSON extended module support for gremlin-python

["aboudreault (GitHub)" <gi...@apache.org>]

That is correct. Note that this serializer function is used by the BigIntegerIO as well. So otherwise.. we would have no way to write a `gx:BigInteger`.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1060 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org