xiazcy commented on code in PR #1788:
URL: https://github.com/apache/tinkerpop/pull/1788#discussion_r961266555
##########
docker/gremlin-server/gremlin-server-integration.yaml:
##########
@@ -18,13 +18,15 @@
host: 0.0.0.0
port: 45940
evaluationTimeout: 30000
+channelizer: org.apache.tinkerpop.gremlin.server.channel.WsAndHttpChannelizer
graphs: {
- graph: conf/tinkergraph-empty.properties,
- classic: conf/tinkergraph-empty.properties,
- modern: conf/tinkergraph-empty.properties,
- crew: conf/tinkergraph-empty.properties,
- grateful: conf/tinkergraph-empty.properties,
- sink: conf/tinkergraph-empty.properties
+ graph: scripts/tinkergraph-empty.properties,
Review Comment:
Hi @spmallette, I dug a bit into to see why `INTEGER` was needed for the
javascript tests, and it looks like it's due to javascript deserializing `LONG`
type ids into its native `Number` type, and storing the vertex ID as a `Number`:
```
getting vertex object from server
{
'@type': 'g:Vertex',
'@value': { id: { '@type': 'g:Int64', '@value': 6 }, label: 'person' }
}
deserialized vertex id is of type
number
```
So when the cucumber test queries the graph by the vertex, javascript
serializes the `Number` vertex ID as an `INTEGER`, and since TinkerGraph
requires the exact type of vertex IDs to match(i.e. a `LONG`), we get empty
result and failed tests:
```
serialize vertex id is of type
number
serializing this
1
serializing this
{ '@type': 'g:Vertex', '@value': { id: 1, label: 'person' } }
serializing this
{ '@type': 'g:Bytecode', '@value': { step: [ [Array], [Array] ] } }
listing result for
g.V(v1).bothE()
[]
F-
```
So this forces javascript tests to use a server configuration of
`gremlin.tinkergraph.vertexIdManager=INTEGER` or
`gremlin.tinkergraph.vertexIdManager=ANY` in order to pass the cucumber tests.
The same failure will occur with GraphBinary as the logic runs the same.
This looks specific to javascript due to the Number typing. I’ve tested this
with the go driver to confirm, which passes cucumber tests with either `LONG`
or `INTEGER` vertexIdManager, as it deserializes the vertex IDs into matching
types.
Given we have made a specific `.properties` file for these tests, I'd assume
this has been a known issue. So I'm wondering if we want to leave this issue
alone for now, and just created a `conf/tinkergraph-service.properties` with
`gremlin.tinkergraph.vertexIdManager=INTEGER` for javascript? Or do we want to
resolve this typing issue in javascript first, so that we can use the default
`.properties` file with `gremlin.tinkergraph.vertexIdManager=LONG`, and save
from having to create the extra `.properties` file?
I'm assuming we want to keep the `gremlin.tinkergraph.vertexIdManager=LONG`
in the default `.properties` file for packaging as per this
[commit](https://github.com/apache/tinkerpop/commit/8d10b81b8635d7ee256021256f6e9384da6c8744)
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
